Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch replaces FrameIdMap and CallStackIdMap with
IndexedMemProfData, which comes with recently introduced methods like
addFrame and addCallStack.

This patch replaces FrameIdMap and CallStackIdMap with
IndexedMemProfData, which comes with recently introduced methods like
addFrame and addCallStack.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 7, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

This patch replaces FrameIdMap and CallStackIdMap with
IndexedMemProfData, which comes with recently introduced methods like
addFrame and addCallStack.


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

1 Files Affected:

  • (modified) llvm/unittests/ProfileData/MemProfTest.cpp (+14-13)
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 74a6acf9e9a82f..fbf74018f8e05d 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -36,6 +36,7 @@ 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;
@@ -500,25 +501,24 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
   // Verify that MemProfRecord can be constructed from IndexedMemProfRecord with
   // CallStackIds only.
 
-  llvm::DenseMap<FrameId, Frame> FrameIdMap;
+  IndexedMemProfData MemProfData;
   Frame F1(1, 0, 0, false);
   Frame F2(2, 0, 0, false);
   Frame F3(3, 0, 0, false);
   Frame F4(4, 0, 0, false);
-  FrameIdMap.insert({F1.hash(), F1});
-  FrameIdMap.insert({F2.hash(), F2});
-  FrameIdMap.insert({F3.hash(), F3});
-  FrameIdMap.insert({F4.hash(), F4});
+  MemProfData.addFrame(F1);
+  MemProfData.addFrame(F2);
+  MemProfData.addFrame(F3);
+  MemProfData.addFrame(F4);
 
-  llvm::DenseMap<CallStackId, llvm::SmallVector<FrameId>> CallStackIdMap;
   llvm::SmallVector<FrameId> CS1 = {F1.hash(), F2.hash()};
   llvm::SmallVector<FrameId> CS2 = {F1.hash(), F3.hash()};
   llvm::SmallVector<FrameId> CS3 = {F2.hash(), F3.hash()};
   llvm::SmallVector<FrameId> CS4 = {F2.hash(), F4.hash()};
-  CallStackIdMap.insert({hashCallStack(CS1), CS1});
-  CallStackIdMap.insert({hashCallStack(CS2), CS2});
-  CallStackIdMap.insert({hashCallStack(CS3), CS3});
-  CallStackIdMap.insert({hashCallStack(CS4), CS4});
+  MemProfData.addCallStack(CS1);
+  MemProfData.addCallStack(CS2);
+  MemProfData.addCallStack(CS3);
+  MemProfData.addCallStack(CS4);
 
   IndexedMemProfRecord IndexedRecord;
   IndexedAllocationInfo AI;
@@ -529,9 +529,10 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
   IndexedRecord.CallSiteIds.push_back(hashCallStack(CS3));
   IndexedRecord.CallSiteIds.push_back(hashCallStack(CS4));
 
-  llvm::memprof::FrameIdConverter<decltype(FrameIdMap)> FrameIdConv(FrameIdMap);
-  llvm::memprof::CallStackIdConverter<decltype(CallStackIdMap)> CSIdConv(
-      CallStackIdMap, FrameIdConv);
+  llvm::memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
+      MemProfData.Frames);
+  llvm::memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)>
+      CSIdConv(MemProfData.CallStacks, FrameIdConv);
 
   MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
 

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 - "Use IndexedMemProfData in tests" for the title?

@kazutakahirata kazutakahirata changed the title [memprof] Use IndexedMemProfData (NFC) [memprof] Use IndexedMemProfData in tests (NFC) Dec 7, 2024
@kazutakahirata kazutakahirata merged commit 00090ac into llvm:main Dec 7, 2024
10 checks passed
@kazutakahirata kazutakahirata deleted the memprof_use_IndexedMemProfData branch December 7, 2024 06:46
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