[memprof] Print out the summary in YAML format.

[snehasish]

Summary

Print out the profile summary in YAML format to make it easier to for
tools and tests to read in the contents of the raw profile.

[tejohnson]

llvm/lib/ProfileData/RawMemProfReader.cpp

106–107

Add a comment here and/or at the declaration that this is printing in YAML format.

llvm/tools/llvm-profdata/llvm-profdata.cpp

2491

Can this be moved into printSummaries?

[snehasish]

Address comments.

[snehasish]

PTAL, thanks!

I would really like to have a test to ensure that the YAML we generate is valid but it seems hard to implement without defining a document type to parse into as a unittest (see https://llvm.org/docs/YamlIO.html#input). Alternatively, I could also extend the lit based test to check if YAML is available like this usage in MLIR (https://git.io/J9ORA). Any suggestions?

llvm/lib/ProfileData/RawMemProfReader.cpp

106–107

Documented the format in the header.

llvm/tools/llvm-profdata/llvm-profdata.cpp

2491

Added a new printYAML method and moved the `OS << "memprof_profile:" statement there.

[tejohnson]

In D116783#3228553, @snehasish wrote:

PTAL, thanks!

I would really like to have a test to ensure that the YAML we generate is valid but it seems hard to implement without defining a document type to parse into as a unittest (see https://llvm.org/docs/YamlIO.html#input). Alternatively, I could also extend the lit based test to check if YAML is available like this usage in MLIR (https://git.io/J9ORA). Any suggestions?

I don't have a strong feeling. Looks like the tests that dump opt remarks to yaml format just manually check that the output lines look as expected but don't attempt to parse them via yaml itself.

llvm/tools/llvm-profdata/llvm-profdata.cpp

2491

Is there a reason for not wanting to put it at the top of printSummaries?

[snehasish]

Convert the snake case yaml to camel case with initcap (matching llvm variable naming convention). This makes it easy to use macro based expansion to generate the meminfoblock contents in subsequent patches.

[snehasish]

In D116783#3228598, @tejohnson wrote:

In D116783#3228553, @snehasish wrote:

PTAL, thanks!

I would really like to have a test to ensure that the YAML we generate is valid but it seems hard to implement without defining a document type to parse into as a unittest (see https://llvm.org/docs/YamlIO.html#input). Alternatively, I could also extend the lit based test to check if YAML is available like this usage in MLIR (https://git.io/J9ORA). Any suggestions?

I don't have a strong feeling. Looks like the tests that dump opt remarks to yaml format just manually check that the output lines look as expected but don't attempt to parse them via yaml itself.

Ok, let me punt on this for now.

I've changed the YAML naming from snake case to match the LLVM variable naming style to keep it consistent with the output from the macro based implementation in D117256.

PTAL, thanks!

llvm/tools/llvm-profdata/llvm-profdata.cpp

2491

Yes, the MemprofProfile marks the outermost object which holds the rest of the information incl. the header information (from printSummaries) and the contents of the raw profile (functionality added in D116784). I decided to introduce the printYAML method in this patch rather than have the print inside printSummaries and then pull it out into a separate method in the next one where we print out the contents of the profile. LMK if you feel strongly about keeping it in the printSummaries method.

[tejohnson]

lgtm

llvm/tools/llvm-profdata/llvm-profdata.cpp

2491

Ah, I missed the fact that D116784 adds more to the outer printYAML. This looks fine then.

llvm/include/llvm/ProfileData/RawMemProfReader.h

 
 namespace llvm {
 namespace memprof {
 
 class RawMemProfReader {
 public:
   RawMemProfReader(std::unique_ptr<MemoryBuffer> DataBuffer)
       : DataBuffer(std::move(DataBuffer)) {}
-  // Prints aggregate counts for each raw profile parsed from the DataBuffer.
-  void printSummaries(raw_ostream &OS) const;
+  // Prints the contents of the profile in YAML format.
+  void printYAML(raw_ostream &OS);
 
   // Return true if the \p DataBuffer starts with magic bytes indicating it is
   // a raw binary memprof profile.
   static bool hasFormat(const MemoryBuffer &DataBuffer);
 
   // Create a RawMemProfReader after sanity checking the contents of the file at
   // \p Path.
   static Expected<std::unique_ptr<RawMemProfReader>> create(const Twine &Path);
 
 private:
+  // Prints aggregate counts for each raw profile parsed from the DataBuffer in
+  // YAML format.
+  void printSummaries(raw_ostream &OS) const;
+
   std::unique_ptr<MemoryBuffer> DataBuffer;
 };
 
 } // namespace memprof
 } // namespace llvm
 
 #endif // LLVM_PROFILEDATA_RAWMEMPROFREADER_H_

llvm/lib/ProfileData/RawMemProfReader.cpp

bool RawMemProfReader::hasFormat(const MemoryBuffer &Buffer) {
   if (Buffer.getBufferSize() < sizeof(uint64_t))
     return false;
   // Aligned read to sanity check that the buffer was allocated with at least 8b
   // alignment.
   const uint64_t Magic = alignedRead(Buffer.getBufferStart());
   return Magic == MEMPROF_RAW_MAGIC_64;
 }
 
+void RawMemProfReader::printYAML(raw_ostream &OS) {
+  OS << "MemprofProfile:\n";
+  printSummaries(OS);
+}
+
 void RawMemProfReader::printSummaries(raw_ostream &OS) const {
-  int Count = 0;
   const char *Next = DataBuffer->getBufferStart();
   while (Next < DataBuffer->getBufferEnd()) {
     auto Summary = computeSummary(Next);
-    OS << "MemProf Profile " << ++Count << "\n";
-    OS << "  Version: " << Summary.Version << "\n";
-    OS << "  TotalSizeBytes: " << Summary.TotalSizeBytes << "\n";
-    OS << "  NumSegments: " << Summary.NumSegments << "\n";
-    OS << "  NumMIBInfo: " << Summary.NumMIBInfo << "\n";
-    OS << "  NumStackOffsets: " << Summary.NumStackOffsets << "\n";
+    OS << "  -\n";
+    OS << "  Header:\n";
+    OS << "    Version: " << Summary.Version << "\n";
+    OS << "    TotalSizeBytes: " << Summary.TotalSizeBytes << "\n";
+    OS << "    NumSegments: " << Summary.NumSegments << "\n";
+    OS << "    NumMibInfo: " << Summary.NumMIBInfo << "\n";
+    OS << "    NumStackOffsets: " << Summary.NumStackOffsets << "\n";
     // TODO: Print the build ids once we can record them using the
     // sanitizer_procmaps library for linux.
 
     auto *H = reinterpret_cast<const Header *>(Next);
     Next += H->TotalSize;
   }
 }
 
 } // namespace memprof
 } // namespace llvm

llvm/test/tools/llvm-profdata/memprof-basic.test

 env MEMPROF_OPTIONS=log_path=stdout ./rawprofile.out > basic.memprofraw
 ```
 
 RUN: llvm-profdata show --memory %p/Inputs/basic.memprofraw -o - | FileCheck %s
 
 We expect 3 MIB entries, 1 each for the malloc calls in the program and one
 additional entry from a realloc in glibc/libio/vasprintf.c.
 
-CHECK: MemProf Profile 1
-CHECK:   Version: 1
-CHECK:   TotalSizeBytes: 1016
-CHECK:   NumSegments: 9
-CHECK:   NumMIBInfo: 3
-CHECK:   NumStackOffsets: 3
+CHECK: MemprofProfile:
+CHECK-NEXT:   -
+CHECK-NEXT:   Header:
+CHECK-NEXT:     Version: 1
+CHECK-NEXT:     TotalSizeBytes: 1016
+CHECK-NEXT:     NumSegments: 9
+CHECK-NEXT:     NumMibInfo: 3
+CHECK-NEXT:     NumStackOffsets: 3

llvm/test/tools/llvm-profdata/memprof-multi.test

 env MEMPROF_OPTIONS=log_path=stdout ./rawprofile.out > multi.memprofraw
 ```
 
 RUN: llvm-profdata show --memory %p/Inputs/multi.memprofraw -o - | FileCheck %s
 
 We expect 2 MIB entries, 1 each for the malloc calls in the program. Unlike the
 memprof-basic.test we do not see any allocation from glibc.
 
-CHECK: MemProf Profile 1
-CHECK:   Version: 1
-CHECK:   TotalSizeBytes: 864
-CHECK:   NumSegments: 9
-CHECK:   NumMIBInfo: 2
-CHECK:   NumStackOffsets: 2
-CHECK: MemProf Profile 2
-CHECK:   Version: 1
-CHECK:   TotalSizeBytes: 864
-CHECK:   NumSegments: 9
-CHECK:   NumMIBInfo: 2
-CHECK:   NumStackOffsets: 2
+CHECK: MemprofProfile:
+CHECK-NEXT:   -
+CHECK-NEXT:   Header:
+CHECK-NEXT:     Version: 1
+CHECK-NEXT:     TotalSizeBytes: 864
+CHECK-NEXT:     NumSegments: 9
+CHECK-NEXT:     NumMibInfo: 2
+CHECK-NEXT:     NumStackOffsets: 2
+CHECK-NEXT:   -
+CHECK-NEXT:   Header:
+CHECK-NEXT:     Version: 1
+CHECK-NEXT:     TotalSizeBytes: 864
+CHECK-NEXT:     NumSegments: 9
+CHECK-NEXT:     NumMibInfo: 2
+CHECK-NEXT:     NumStackOffsets: 2

llvm/tools/llvm-profdata/llvm-profdata.cpp

 
 static int showMemProfProfile(const std::string &Filename, raw_fd_ostream &OS) {
   auto ReaderOr = llvm::memprof::RawMemProfReader::create(Filename);
   if (Error E = ReaderOr.takeError())
     exitWithError(std::move(E), Filename);
 
   std::unique_ptr<llvm::memprof::RawMemProfReader> Reader(
       ReaderOr.get().release());
-  Reader->printSummaries(OS);
+
+  Reader->printYAML(OS);
   return 0;
 }
 
 static int showDebugInfoCorrelation(const std::string &Filename,
                                     bool ShowDetailedSummary,
                                     bool ShowProfileSymbolList,
                                     raw_fd_ostream &OS) {
   std::unique_ptr<InstrProfCorrelator> Correlator;