Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch adds a helper function to replace an idiom like:

CallStackId CSId = hashCallStack(CallStack)
MemProfData.CallStacks.try_emplace(CSId, CallStack);
// Do something with CSId.

This patch adds a helper function to replace an idiom like:

  CallStackId CSId = hashCallStack(CallStack)
  MemProfData.CallStacks.try_emplace(CSId, CallStack);
  // Do something with CSId.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Dec 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2024

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds a helper function to replace an idiom like:

CallStackId CSId = hashCallStack(CallStack)
MemProfData.CallStacks.try_emplace(CSId, CallStack);
// Do something with CSId.


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

3 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/MemProf.h (+12)
  • (modified) llvm/lib/ProfileData/MemProfReader.cpp (+4-10)
  • (modified) llvm/unittests/ProfileData/MemProfTest.cpp (+2-4)
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 51f012cdaed3ac..8a85196e57044d 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -1029,6 +1029,18 @@ struct IndexedMemProfData {
     Frames.try_emplace(Id, F);
     return Id;
   }
+
+  CallStackId addCallStack(ArrayRef<FrameId> CS) {
+    CallStackId CSId = hashCallStack(CS);
+    CallStacks.try_emplace(CSId, CS);
+    return CSId;
+  }
+
+  CallStackId addCallStack(SmallVector<FrameId> &&CS) {
+    CallStackId CSId = hashCallStack(CS);
+    CallStacks.try_emplace(CSId, std::move(CS));
+    return CSId;
+  }
 };
 
 struct FrameStat {
diff --git a/llvm/lib/ProfileData/MemProfReader.cpp b/llvm/lib/ProfileData/MemProfReader.cpp
index 9dacf298985937..2cf0e9c74c0d23 100644
--- a/llvm/lib/ProfileData/MemProfReader.cpp
+++ b/llvm/lib/ProfileData/MemProfReader.cpp
@@ -500,8 +500,7 @@ Error RawMemProfReader::mapRawProfileToRecords() {
       Callstack.append(Frames.begin(), Frames.end());
     }
 
-    CallStackId CSId = hashCallStack(Callstack);
-    MemProfData.CallStacks.insert({CSId, Callstack});
+    CallStackId CSId = MemProfData.addCallStack(Callstack);
 
     // We attach the memprof record to each function bottom-up including the
     // first non-inline frame.
@@ -520,11 +519,8 @@ Error RawMemProfReader::mapRawProfileToRecords() {
     // Some functions may have only callsite data and no allocation data. Here
     // we insert a new entry for callsite data if we need to.
     IndexedMemProfRecord &Record = MemProfData.Records[Id];
-    for (LocationPtr Loc : Locs) {
-      CallStackId CSId = hashCallStack(*Loc);
-      MemProfData.CallStacks.insert({CSId, *Loc});
-      Record.CallSiteIds.push_back(CSId);
-    }
+    for (LocationPtr Loc : Locs)
+      Record.CallSiteIds.push_back(MemProfData.addCallStack(*Loc));
   }
 
   return Error::success();
@@ -769,9 +765,7 @@ void YAMLMemProfReader::parse(StringRef YAMLData) {
     IndexedCallStack.reserve(CallStack.size());
     for (const Frame &F : CallStack)
       IndexedCallStack.push_back(MemProfData.addFrame(F));
-    CallStackId CSId = hashCallStack(IndexedCallStack);
-    MemProfData.CallStacks.try_emplace(CSId, std::move(IndexedCallStack));
-    return CSId;
+    return MemProfData.addCallStack(std::move(IndexedCallStack));
   };
 
   for (const auto &[GUID, Record] : Doc.HeapProfileRecords) {
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index f0567d0707fef0..74a6acf9e9a82f 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -434,8 +434,7 @@ TEST(MemProf, BaseMemProfReader) {
   MemProfData.addFrame(F2);
 
   llvm::SmallVector<FrameId> CallStack{F1.hash(), F2.hash()};
-  CallStackId CSId = hashCallStack(CallStack);
-  MemProfData.CallStacks.try_emplace(CSId, CallStack);
+  CallStackId CSId = MemProfData.addCallStack(std::move(CallStack));
 
   IndexedMemProfRecord FakeRecord;
   MemInfoBlock Block;
@@ -470,8 +469,7 @@ TEST(MemProf, BaseMemProfReaderWithCSIdMap) {
   MemProfData.addFrame(F2);
 
   llvm::SmallVector<FrameId> CallStack = {F1.hash(), F2.hash()};
-  CallStackId CSId = hashCallStack(CallStack);
-  MemProfData.CallStacks.insert({CSId, CallStack});
+  MemProfData.addCallStack(CallStack);
 
   IndexedMemProfRecord FakeRecord;
   MemInfoBlock Block;

@kazutakahirata kazutakahirata merged commit c5e4e8f into llvm:main Dec 6, 2024
10 checks passed
@kazutakahirata kazutakahirata deleted the memprof_addCallStack branch December 6, 2024 20:10
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