[llvm] e699904 - Revert "[memprof] Introduce a wrapper around MemInfoBlock."

Snehasish Kumar via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 14 11:43:44 PST 2022


Author: Snehasish Kumar
Date: 2022-02-14T11:42:58-08:00
New Revision: e6999040f5758f89a64b6232119b775b7bd1c85b

URL: https://github.com/llvm/llvm-project/commit/e6999040f5758f89a64b6232119b775b7bd1c85b
DIFF: https://github.com/llvm/llvm-project/commit/e6999040f5758f89a64b6232119b775b7bd1c85b.diff

LOG: Revert "[memprof] Introduce a wrapper around MemInfoBlock."

This reverts commit 9b67165285c5e752fce3b554769f5a22e7b38da8. [3/4]

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/MemProf.h
    llvm/lib/ProfileData/RawMemProfReader.cpp
    llvm/test/tools/llvm-profdata/memprof-basic.test
    llvm/unittests/ProfileData/MemProfTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 2fa577a626bbe..1c92ec5d8892e 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -5,134 +5,12 @@
 #include <string>
 #include <vector>
 
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/ProfileData/MemProfData.inc"
-#include "llvm/ProfileData/ProfileCommon.h"
-#include "llvm/Support/Endian.h"
-#include "llvm/Support/EndianStream.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 namespace memprof {
 
-enum class Meta : uint64_t {
-  Start = 0,
-#define MIBEntryDef(NameTag, Name, Type) NameTag,
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-  Size
-};
-
-using MemProfSchema = llvm::SmallVector<Meta, static_cast<int>(Meta::Size)>;
-
-// Holds the actual MemInfoBlock data with all fields. Contents may be read or
-// written partially by providing an appropriate schema to the serialize and
-// deserialize methods.
-struct PortableMemInfoBlock {
-  PortableMemInfoBlock() = default;
-  explicit PortableMemInfoBlock(const MemInfoBlock &Block) {
-#define MIBEntryDef(NameTag, Name, Type) Name = Block.Name;
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-  }
-
-  PortableMemInfoBlock(const MemProfSchema &Schema, const unsigned char *Ptr) {
-    deserialize(Schema, Ptr);
-  }
-
-  // Read the contents of \p Ptr based on the \p Schema to populate the
-  // MemInfoBlock member.
-  void deserialize(const MemProfSchema &Schema, const unsigned char *Ptr) {
-    using namespace support;
-
-    for (const Meta Id : Schema) {
-      switch (Id) {
-#define MIBEntryDef(NameTag, Name, Type)                                       \
-  case Meta::Name: {                                                           \
-    Name = endian::readNext<Type, little, unaligned>(Ptr);                     \
-  } break;
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-      default:
-        llvm_unreachable("Unknown meta type id, is the profile collected from "
-                         "a newer version of the runtime?");
-      }
-    }
-  }
-
-  // Write the contents of the MemInfoBlock based on the \p Schema provided to
-  // the raw_ostream \p OS.
-  void serialize(const MemProfSchema &Schema, raw_ostream &OS) const {
-    using namespace support;
-
-    endian::Writer LE(OS, little);
-    for (const Meta Id : Schema) {
-      switch (Id) {
-#define MIBEntryDef(NameTag, Name, Type)                                       \
-  case Meta::Name: {                                                           \
-    LE.write<Type>(Name);                                                      \
-  } break;
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-      default:
-        llvm_unreachable("Unknown meta type id, invalid input?");
-      }
-    }
-  }
-
-  // Print out the contents of the MemInfoBlock in YAML format.
-  void printYAML(raw_ostream &OS) const {
-    OS << "    MemInfoBlock:\n";
-#define MIBEntryDef(NameTag, Name, Type)                                       \
-  OS << "      " << #Name << ": " << Name << "\n";
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-  }
-
-  // Define getters for each type which can be called by analyses.
-#define MIBEntryDef(NameTag, Name, Type)                                       \
-  Type get##Name() const { return Name; }
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-
-  void clear() { *this = PortableMemInfoBlock(); }
-
-  // Returns the full schema currently in use.
-  static MemProfSchema getSchema() {
-    MemProfSchema List;
-#define MIBEntryDef(NameTag, Name, Type) List.push_back(Meta::Name);
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-    return List;
-  }
-
-  bool operator==(const PortableMemInfoBlock &Other) const {
-#define MIBEntryDef(NameTag, Name, Type)                                       \
-  if (Other.get##Name() != get##Name())                                        \
-    return false;
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-    return true;
-  }
-
-  bool operator!=(const PortableMemInfoBlock &Other) const {
-    return !operator==(Other);
-  }
-
-  static constexpr size_t serializedSize() {
-    size_t Result = 0;
-#define MIBEntryDef(NameTag, Name, Type) Result += sizeof(Type);
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-    return Result;
-  }
-
-private:
-#define MIBEntryDef(NameTag, Name, Type) Type Name = Type();
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-};
-
 struct MemProfRecord {
   struct Frame {
     std::string Function;
@@ -146,11 +24,14 @@ struct MemProfRecord {
   };
 
   std::vector<Frame> CallStack;
-  PortableMemInfoBlock Info;
+  // TODO: Replace this with the entry format described in the RFC so
+  // that the InstrProfRecord reader and writer do not have to be concerned
+  // about backwards compat.
+  MemInfoBlock Info;
 
   void clear() {
     CallStack.clear();
-    Info.clear();
+    Info = MemInfoBlock();
   }
 
   // Prints out the contents of the memprof record in YAML.
@@ -166,7 +47,45 @@ struct MemProfRecord {
          << "      Inline: " << Frame.IsInlineFrame << "\n";
     }
 
-    Info.printYAML(OS);
+    OS << "    MemInfoBlock:\n";
+
+    // TODO: Replace this once the format is updated to be version agnostic.
+    OS << "      "
+       << "AllocCount: " << Info.AllocCount << "\n";
+    OS << "      "
+       << "TotalAccessCount: " << Info.TotalAccessCount << "\n";
+    OS << "      "
+       << "MinAccessCount: " << Info.MinAccessCount << "\n";
+    OS << "      "
+       << "MaxAccessCount: " << Info.MaxAccessCount << "\n";
+    OS << "      "
+       << "TotalSize: " << Info.TotalSize << "\n";
+    OS << "      "
+       << "MinSize: " << Info.MinSize << "\n";
+    OS << "      "
+       << "MaxSize: " << Info.MaxSize << "\n";
+    OS << "      "
+       << "AllocTimestamp: " << Info.AllocTimestamp << "\n";
+    OS << "      "
+       << "DeallocTimestamp: " << Info.DeallocTimestamp << "\n";
+    OS << "      "
+       << "TotalLifetime: " << Info.TotalLifetime << "\n";
+    OS << "      "
+       << "MinLifetime: " << Info.MinLifetime << "\n";
+    OS << "      "
+       << "MaxLifetime: " << Info.MaxLifetime << "\n";
+    OS << "      "
+       << "AllocCpuId: " << Info.AllocCpuId << "\n";
+    OS << "      "
+       << "DeallocCpuId: " << Info.DeallocCpuId << "\n";
+    OS << "      "
+       << "NumMigratedCpu: " << Info.NumMigratedCpu << "\n";
+    OS << "      "
+       << "NumLifetimeOverlaps: " << Info.NumLifetimeOverlaps << "\n";
+    OS << "      "
+       << "NumSameAllocCpu: " << Info.NumSameAllocCpu << "\n";
+    OS << "      "
+       << "NumSameDeallocCpu: " << Info.NumSameDeallocCpu << "\n";
   }
 };
 

diff  --git a/llvm/lib/ProfileData/RawMemProfReader.cpp b/llvm/lib/ProfileData/RawMemProfReader.cpp
index 43ef7c947366a..335ec04bc0980 100644
--- a/llvm/lib/ProfileData/RawMemProfReader.cpp
+++ b/llvm/lib/ProfileData/RawMemProfReader.cpp
@@ -368,7 +368,7 @@ Error RawMemProfReader::fillRecord(const uint64_t Id, const MemInfoBlock &MIB,
           I != 0);
     }
   }
-  Record.Info = PortableMemInfoBlock(MIB);
+  Record.Info = MIB;
   return Error::success();
 }
 

diff  --git a/llvm/test/tools/llvm-profdata/memprof-basic.test b/llvm/test/tools/llvm-profdata/memprof-basic.test
index ccb5b4a438796..23c35c28ae96d 100644
--- a/llvm/test/tools/llvm-profdata/memprof-basic.test
+++ b/llvm/test/tools/llvm-profdata/memprof-basic.test
@@ -76,7 +76,6 @@ CHECK-NEXT:       NumMigratedCpu: 1
 CHECK-NEXT:       NumLifetimeOverlaps: 0
 CHECK-NEXT:       NumSameAllocCpu: 0
 CHECK-NEXT:       NumSameDeallocCpu: 0
-CHECK-NEXT:       DataTypeId: {{[0-9]+}}
 CHECK-NEXT:   -
 CHECK-NEXT:     Callstack:
 CHECK-NEXT:     -
@@ -113,7 +112,6 @@ CHECK-NEXT:       NumMigratedCpu: 0
 CHECK-NEXT:       NumLifetimeOverlaps: 0
 CHECK-NEXT:       NumSameAllocCpu: 0
 CHECK-NEXT:       NumSameDeallocCpu: 0
-CHECK-NEXT:       DataTypeId: {{[0-9]+}}
 CHECK-NEXT:   -
 CHECK-NEXT:     Callstack:
 CHECK-NEXT:     -
@@ -150,4 +148,3 @@ CHECK-NEXT:       NumMigratedCpu: 0
 CHECK-NEXT:       NumLifetimeOverlaps: 0
 CHECK-NEXT:       NumSameAllocCpu: 0
 CHECK-NEXT:       NumSameDeallocCpu: 0
-CHECK-NEXT:       DataTypeId: {{[0-9]+}}

diff  --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 38dc8630f18bb..09ffe8fd634f2 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -9,7 +9,6 @@
 #include "llvm/ProfileData/RawMemProfReader.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MD5.h"
-#include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -25,9 +24,6 @@ using ::llvm::DILocal;
 using ::llvm::memprof::CallStackMap;
 using ::llvm::memprof::MemInfoBlock;
 using ::llvm::memprof::MemProfRecord;
-using ::llvm::memprof::MemProfSchema;
-using ::llvm::memprof::Meta;
-using ::llvm::memprof::PortableMemInfoBlock;
 using ::llvm::memprof::RawMemProfReader;
 using ::llvm::memprof::SegmentEntry;
 using ::llvm::object::SectionedAddress;
@@ -103,14 +99,6 @@ MATCHER_P4(FrameContains, Function, LineOffset, Column, Inline, "") {
   return false;
 }
 
-MemProfSchema getFullSchema() {
-  MemProfSchema Schema;
-#define MIBEntryDef(NameTag, Name, Type) Schema.push_back(Meta::Name);
-#include "llvm/ProfileData/MIBEntryDef.inc"
-#undef MIBEntryDef
-  return Schema;
-}
-
 TEST(MemProf, FillsValue) {
   std::unique_ptr<MockSymbolizer> Symbolizer(new MockSymbolizer());
 
@@ -148,8 +136,8 @@ TEST(MemProf, FillsValue) {
   }
   EXPECT_EQ(Records.size(), 2U);
 
-  EXPECT_EQ(Records[0].Info.getAllocCount(), 1U);
-  EXPECT_EQ(Records[1].Info.getAllocCount(), 2U);
+  EXPECT_EQ(Records[0].Info.AllocCount, 1U);
+  EXPECT_EQ(Records[1].Info.AllocCount, 2U);
   EXPECT_THAT(Records[0].CallStack[0], FrameContains("foo", 5U, 30U, false));
   EXPECT_THAT(Records[0].CallStack[1], FrameContains("bar", 51U, 20U, true));
 
@@ -159,29 +147,4 @@ TEST(MemProf, FillsValue) {
   EXPECT_THAT(Records[1].CallStack[3], FrameContains("bar", 51U, 20U, true));
 }
 
-TEST(MemProf, PortableWrapper) {
-  MemInfoBlock Info(/*size=*/16, /*access_count=*/7, /*alloc_timestamp=*/1000,
-                    /*dealloc_timestamp=*/2000, /*alloc_cpu=*/3,
-                    /*dealloc_cpu=*/4);
-
-  const auto Schema = getFullSchema();
-  PortableMemInfoBlock WriteBlock(Info);
-
-  std::string Buffer;
-  llvm::raw_string_ostream OS(Buffer);
-  WriteBlock.serialize(Schema, OS);
-  OS.flush();
-
-  PortableMemInfoBlock ReadBlock(
-      Schema, reinterpret_cast<const unsigned char *>(Buffer.data()));
-
-  EXPECT_EQ(ReadBlock, WriteBlock);
-  // Here we compare directly with the actual counts instead of MemInfoBlock
-  // members. Since the MemInfoBlock struct is packed and the EXPECT_EQ macros
-  // take a reference to the params, this results in unaligned accesses.
-  EXPECT_EQ(1, ReadBlock.getAllocCount());
-  EXPECT_EQ(7, ReadBlock.getTotalAccessCount());
-  EXPECT_EQ(3, ReadBlock.getAllocCpuId());
-}
-
 } // namespace


        


More information about the llvm-commits mailing list