Skip to content

[memprof] Add IndexedMemProfData::addCallStack #118920

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

Merged
merged 1 commit into from
Dec 6, 2024

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