Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[memprof] Use namespaces in a unit test #119144

Merged

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
Contributor

@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

llvm/unittests/ProfileData/MemProfTest.cpp Outdated Show resolved Hide resolved
@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
broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024
MemProfTest.cpp is about MemProf, so mentioning llvm::memprof
everywhere is quite verbose.
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