Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

MemProfTest.cpp is about MemProf, so mentioning llvm::memprof
everywhere is quite verbose.

MemProfTest.cpp is about MemProf, so mentioning llvm::memprof
everywhere is quite verbose.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 8, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

MemProfTest.cpp is about MemProf, so mentioning llvm::memprof
everywhere is quite verbose.


Full diff: https://github.com/llvm/llvm-project/pull/119144.diff

1 Files Affected:

  • (modified) llvm/unittests/ProfileData/MemProfTest.cpp (+37-56)
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 4edd5d449072cf..ecc16f812764d1 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -30,22 +30,6 @@ using ::llvm::DILineInfo;
 using ::llvm::DILineInfoSpecifier;
 using ::llvm::DILocal;
 using ::llvm::StringRef;
-using ::llvm::memprof::CallStackId;
-using ::llvm::memprof::CallStackMap;
-using ::llvm::memprof::Frame;
-using ::llvm::memprof::FrameId;
-using ::llvm::memprof::hashCallStack;
-using ::llvm::memprof::IndexedAllocationInfo;
-using ::llvm::memprof::IndexedMemProfData;
-using ::llvm::memprof::IndexedMemProfRecord;
-using ::llvm::memprof::MemInfoBlock;
-using ::llvm::memprof::MemProfReader;
-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;
 using ::llvm::symbolize::SymbolizableModule;
 using ::testing::ElementsAre;
@@ -53,6 +37,7 @@ using ::testing::Pair;
 using ::testing::Return;
 using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
+using namespace ::llvm::memprof;
 
 class MockSymbolizer : public SymbolizableModule {
 public:
@@ -251,7 +236,7 @@ TEST(MemProf, PortableWrapper) {
                     /*dealloc_timestamp=*/2000, /*alloc_cpu=*/3,
                     /*dealloc_cpu=*/4, /*Histogram=*/0, /*HistogramSize=*/0);
 
-  const auto Schema = llvm::memprof::getFullSchema();
+  const auto Schema = getFullSchema();
   PortableMemInfoBlock WriteBlock(Info, Schema);
 
   std::string Buffer;
@@ -271,7 +256,7 @@ TEST(MemProf, PortableWrapper) {
 }
 
 TEST(MemProf, RecordSerializationRoundTripVerion2) {
-  const auto Schema = llvm::memprof::getFullSchema();
+  const auto Schema = getFullSchema();
 
   MemInfoBlock Info(/*size=*/16, /*access_count=*/7, /*alloc_timestamp=*/1000,
                     /*dealloc_timestamp=*/2000, /*alloc_cpu=*/3,
@@ -290,17 +275,16 @@ TEST(MemProf, RecordSerializationRoundTripVerion2) {
 
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
-  Record.serialize(Schema, OS, llvm::memprof::Version2);
+  Record.serialize(Schema, OS, Version2);
 
   const IndexedMemProfRecord GotRecord = IndexedMemProfRecord::deserialize(
-      Schema, reinterpret_cast<const unsigned char *>(Buffer.data()),
-      llvm::memprof::Version2);
+      Schema, reinterpret_cast<const unsigned char *>(Buffer.data()), Version2);
 
   EXPECT_EQ(Record, GotRecord);
 }
 
 TEST(MemProf, RecordSerializationRoundTripVersion2HotColdSchema) {
-  const auto Schema = llvm::memprof::getHotColdSchema();
+  const auto Schema = getHotColdSchema();
 
   MemInfoBlock Info;
   Info.AllocCount = 11;
@@ -340,11 +324,10 @@ TEST(MemProf, RecordSerializationRoundTripVersion2HotColdSchema) {
 
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
-  Record.serialize(Schema, OS, llvm::memprof::Version2);
+  Record.serialize(Schema, OS, Version2);
 
   const IndexedMemProfRecord GotRecord = IndexedMemProfRecord::deserialize(
-      Schema, reinterpret_cast<const unsigned char *>(Buffer.data()),
-      llvm::memprof::Version2);
+      Schema, reinterpret_cast<const unsigned char *>(Buffer.data()), Version2);
 
   // Verify that Schema comes back correctly after deserialization. Technically,
   // the comparison between Record and GotRecord below includes the comparison
@@ -521,10 +504,10 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
   IndexedRecord.CallSiteIds.push_back(hashCallStack(CS3));
   IndexedRecord.CallSiteIds.push_back(hashCallStack(CS4));
 
-  llvm::memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
+  FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
       MemProfData.Frames);
-  llvm::memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)>
-      CSIdConv(MemProfData.CallStacks, FrameIdConv);
+  CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
+      MemProfData.CallStacks, FrameIdConv);
 
   MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
 
@@ -553,18 +536,17 @@ MemInfoBlock makePartialMIB() {
 TEST(MemProf, MissingCallStackId) {
   // Use a non-existent CallStackId to trigger a mapping error in
   // toMemProfRecord.
-  IndexedAllocationInfo AI(0xdeadbeefU, makePartialMIB(),
-                           llvm::memprof::getHotColdSchema());
+  IndexedAllocationInfo AI(0xdeadbeefU, makePartialMIB(), getHotColdSchema());
 
   IndexedMemProfRecord IndexedMR;
   IndexedMR.AllocSites.push_back(AI);
 
   // Create empty maps.
   IndexedMemProfData MemProfData;
-  llvm::memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
+  FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
       MemProfData.Frames);
-  llvm::memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)>
-      CSIdConv(MemProfData.CallStacks, FrameIdConv);
+  CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
+      MemProfData.CallStacks, FrameIdConv);
 
   // We are only interested in errors, not the return value.
   (void)IndexedMR.toMemProfRecord(CSIdConv);
@@ -575,8 +557,7 @@ TEST(MemProf, MissingCallStackId) {
 }
 
 TEST(MemProf, MissingFrameId) {
-  IndexedAllocationInfo AI(0x222, makePartialMIB(),
-                           llvm::memprof::getHotColdSchema());
+  IndexedAllocationInfo AI(0x222, makePartialMIB(), getHotColdSchema());
 
   IndexedMemProfRecord IndexedMR;
   IndexedMR.AllocSites.push_back(AI);
@@ -585,10 +566,10 @@ TEST(MemProf, MissingFrameId) {
   IndexedMemProfData MemProfData;
   MemProfData.CallStacks.insert({0x222, {2, 3}});
 
-  llvm::memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
+  FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
       MemProfData.Frames);
-  llvm::memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)>
-      CSIdConv(MemProfData.CallStacks, FrameIdConv);
+  CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
+      MemProfData.CallStacks, FrameIdConv);
 
   // We are only interested in errors, not the return value.
   (void)IndexedMR.toMemProfRecord(CSIdConv);
@@ -600,11 +581,11 @@ TEST(MemProf, MissingFrameId) {
 
 // Verify CallStackRadixTreeBuilder can handle empty inputs.
 TEST(MemProf, RadixTreeBuilderEmpty) {
-  llvm::DenseMap<FrameId, llvm::memprof::LinearFrameId> MemProfFrameIndexes;
+  llvm::DenseMap<FrameId, LinearFrameId> MemProfFrameIndexes;
   llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>> MemProfCallStackData;
-  llvm::DenseMap<FrameId, llvm::memprof::FrameStat> FrameHistogram =
-      llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
-  llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
+  llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
+      computeFrameHistogram<FrameId>(MemProfCallStackData);
+  CallStackRadixTreeBuilder<FrameId> Builder;
   Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
                 FrameHistogram);
   ASSERT_THAT(Builder.getRadixArray(), testing::IsEmpty());
@@ -614,14 +595,14 @@ TEST(MemProf, RadixTreeBuilderEmpty) {
 
 // Verify CallStackRadixTreeBuilder can handle one trivial call stack.
 TEST(MemProf, RadixTreeBuilderOne) {
-  llvm::DenseMap<FrameId, llvm::memprof::LinearFrameId> MemProfFrameIndexes = {
+  llvm::DenseMap<FrameId, LinearFrameId> MemProfFrameIndexes = {
       {11, 1}, {12, 2}, {13, 3}};
   llvm::SmallVector<FrameId> CS1 = {13, 12, 11};
   llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>> MemProfCallStackData;
   MemProfCallStackData.insert({hashCallStack(CS1), CS1});
-  llvm::DenseMap<FrameId, llvm::memprof::FrameStat> FrameHistogram =
-      llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
-  llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
+  llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
+      computeFrameHistogram<FrameId>(MemProfCallStackData);
+  CallStackRadixTreeBuilder<FrameId> Builder;
   Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
                 FrameHistogram);
   EXPECT_THAT(Builder.getRadixArray(),
@@ -636,16 +617,16 @@ TEST(MemProf, RadixTreeBuilderOne) {
 
 // Verify CallStackRadixTreeBuilder can form a link between two call stacks.
 TEST(MemProf, RadixTreeBuilderTwo) {
-  llvm::DenseMap<FrameId, llvm::memprof::LinearFrameId> MemProfFrameIndexes = {
+  llvm::DenseMap<FrameId, LinearFrameId> MemProfFrameIndexes = {
       {11, 1}, {12, 2}, {13, 3}};
   llvm::SmallVector<FrameId> CS1 = {12, 11};
   llvm::SmallVector<FrameId> CS2 = {13, 12, 11};
   llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>> MemProfCallStackData;
   MemProfCallStackData.insert({hashCallStack(CS1), CS1});
   MemProfCallStackData.insert({hashCallStack(CS2), CS2});
-  llvm::DenseMap<FrameId, llvm::memprof::FrameStat> FrameHistogram =
-      llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
-  llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
+  llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
+      computeFrameHistogram<FrameId>(MemProfCallStackData);
+  CallStackRadixTreeBuilder<FrameId> Builder;
   Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
                 FrameHistogram);
   EXPECT_THAT(Builder.getRadixArray(),
@@ -664,7 +645,7 @@ TEST(MemProf, RadixTreeBuilderTwo) {
 // Verify CallStackRadixTreeBuilder can form a jump to a prefix that itself has
 // another jump to another prefix.
 TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
-  llvm::DenseMap<FrameId, llvm::memprof::LinearFrameId> MemProfFrameIndexes = {
+  llvm::DenseMap<FrameId, LinearFrameId> MemProfFrameIndexes = {
       {11, 1}, {12, 2}, {13, 3}, {14, 4}, {15, 5}, {16, 6}, {17, 7}, {18, 8},
   };
   llvm::SmallVector<FrameId> CS1 = {14, 13, 12, 11};
@@ -676,9 +657,9 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
   MemProfCallStackData.insert({hashCallStack(CS2), CS2});
   MemProfCallStackData.insert({hashCallStack(CS3), CS3});
   MemProfCallStackData.insert({hashCallStack(CS4), CS4});
-  llvm::DenseMap<FrameId, llvm::memprof::FrameStat> FrameHistogram =
-      llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
-  llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
+  llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
+      computeFrameHistogram<FrameId>(MemProfCallStackData);
+  CallStackRadixTreeBuilder<FrameId> Builder;
   Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
                 FrameHistogram);
   EXPECT_THAT(Builder.getRadixArray(),
@@ -731,7 +712,7 @@ TEST(MemProf, YAMLParser) {
     - {Function: 0x800, LineOffset: 88, Column: 80, IsInlineFrame: false}
 )YAML";
 
-  llvm::memprof::YAMLMemProfReader YAMLReader;
+  YAMLMemProfReader YAMLReader;
   YAMLReader.parse(YAMLData);
   IndexedMemProfData MemProfData = YAMLReader.takeMemProfData();
 
@@ -802,7 +783,7 @@ TEST(MemProf, YAMLWriterMIB) {
   MIB.TotalSize = 222;
   MIB.TotalLifetime = 333;
   MIB.TotalLifetimeAccessDensity = 444;
-  PortableMemInfoBlock PMIB(MIB, llvm::memprof::getHotColdSchema());
+  PortableMemInfoBlock PMIB(MIB, getHotColdSchema());
 
   std::string Out = serializeInYAML(PMIB);
   EXPECT_EQ(Out, R"YAML(---

Copy link

@snehasish snehasish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@kazutakahirata kazutakahirata changed the title [memprof] Use "using namespace ::llvm::memprof;" in a unit test [memprof] Use namespaces in a unit test Dec 8, 2024
@kazutakahirata kazutakahirata merged commit 6a137fb into llvm:main Dec 9, 2024
8 checks passed
@kazutakahirata kazutakahirata deleted the memprof_cleanup_unittest_using_2 branch December 9, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants