[llvm] [memprof] Simplify readMemprof (NFC) (PR #119930)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 13 21:58:38 PST 2024
https://github.com/kazutakahirata updated https://github.com/llvm/llvm-project/pull/119930
>From 9d327649ef879dc9aafa88e4232362d06a982796 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Fri, 13 Dec 2024 12:24:47 -0800
Subject: [PATCH 1/3] [memprof] Simplify readMemprof (NFC)
This patch essentially replaces:
std::pair<const std::vector<Frame> *, unsigned>
with:
ArrayRef<Frame>
This way, we can store and pass ArrayRef<Frame>, conceptually one
item, instead of the pointer and index.
The only problem is that std::set<ArrayRef<Frame>> doesn't work
because ArrayRef doesn't come with operator<. This patch works around
the problem by providing CallStackRef, a thin wrapper around
ArrayRef<Frame>.
---
.../Instrumentation/MemProfiler.cpp | 32 ++++++++++++-------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index ab7d3c70029910..03c9c64e8a1db6 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -763,9 +763,8 @@ static AllocationType addCallStack(CallStackTrie &AllocTrie,
// non-zero.
static bool
stackFrameIncludesInlinedCallStack(ArrayRef<Frame> ProfileCallStack,
- ArrayRef<uint64_t> InlinedCallStack,
- unsigned StartIndex = 0) {
- auto StackFrame = ProfileCallStack.begin() + StartIndex;
+ ArrayRef<uint64_t> InlinedCallStack) {
+ auto StackFrame = ProfileCallStack.begin();
auto InlCallStackIter = InlinedCallStack.begin();
for (; StackFrame != ProfileCallStack.end() &&
InlCallStackIter != InlinedCallStack.end();
@@ -969,10 +968,20 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
// Build maps of the location hash to all profile data with that leaf location
// (allocation info and the callsites).
std::map<uint64_t, std::set<const AllocationInfo *>> LocHashToAllocInfo;
- // For the callsites we need to record the index of the associated frame in
- // the frame array (see comments below where the map entries are added).
- std::map<uint64_t, std::set<std::pair<const std::vector<Frame> *, unsigned>>>
- LocHashToCallSites;
+ // A thin wrapper around ArrayRef<Frame> to facilitate std::set<CallStackRef>.
+ struct CallStackRef : public ArrayRef<Frame> {
+ CallStackRef(ArrayRef<Frame> CS, unsigned Pos)
+ : ArrayRef<Frame>(CS.drop_front(Pos)) {}
+ // std::set requires std::less.
+ bool operator<(const CallStackRef &R) const {
+ const CallStackRef &L = *this;
+ return std::make_pair(L.data(), L.size()) <
+ std::make_pair(R.data(), R.size());
+ }
+ };
+ // For the callsites we need to record slices of the frame array (see comments
+ // below where the map entries are added).
+ std::map<uint64_t, std::set<CallStackRef>> LocHashToCallSites;
for (auto &AI : MemProfRec->AllocSites) {
NumOfMemProfAllocContextProfiles++;
// Associate the allocation info with the leaf frame. The later matching
@@ -989,7 +998,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
unsigned Idx = 0;
for (auto &StackFrame : CS) {
uint64_t StackId = computeStackId(StackFrame);
- LocHashToCallSites[StackId].insert(std::make_pair(&CS, Idx++));
+ LocHashToCallSites[StackId].emplace(CS, Idx++);
ProfileHasColumns |= StackFrame.Column;
// Once we find this function, we can stop recording.
if (StackFrame.Function == FuncGUID)
@@ -1029,8 +1038,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
// and another callsite).
std::map<uint64_t, std::set<const AllocationInfo *>>::iterator
AllocInfoIter;
- std::map<uint64_t, std::set<std::pair<const std::vector<Frame> *,
- unsigned>>>::iterator CallSitesIter;
+ std::map<uint64_t, std::set<CallStackRef>>::iterator CallSitesIter;
for (const DILocation *DIL = I.getDebugLoc(); DIL != nullptr;
DIL = DIL->getInlinedAt()) {
// Use C++ linkage name if possible. Need to compile with
@@ -1121,8 +1129,8 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
for (auto CallStackIdx : CallSitesIter->second) {
// If we found and thus matched all frames on the call, create and
// attach call stack metadata.
- if (stackFrameIncludesInlinedCallStack(
- *CallStackIdx.first, InlinedCallStack, CallStackIdx.second)) {
+ if (stackFrameIncludesInlinedCallStack(CallStackIdx,
+ InlinedCallStack)) {
NumOfMemProfMatchedCallSites++;
addCallsiteMetadata(I, InlinedCallStack, Ctx);
// Only need to find one with a matching call stack and add a single
>From 4dba1363c3dd2c43c4a7ff395b82d4455a4606f4 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Fri, 13 Dec 2024 16:05:46 -0800
Subject: [PATCH 2/3] Address comments.
---
.../Instrumentation/MemProfiler.cpp | 24 ++++++++-----------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 03c9c64e8a1db6..0b310d9c8a01bf 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -725,8 +725,7 @@ static uint64_t computeStackId(const memprof::Frame &Frame) {
// Helper to generate a single hash id for a given callstack, used for emitting
// matching statistics and useful for uniquing such statistics across modules.
-static uint64_t
-computeFullStackId(const std::vector<memprof::Frame> &CallStack) {
+static uint64_t computeFullStackId(ArrayRef<Frame> CallStack) {
llvm::HashBuilder<llvm::TruncatedBLAKE3<8>, llvm::endianness::little>
HashBuilder;
for (auto &F : CallStack)
@@ -968,20 +967,16 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
// Build maps of the location hash to all profile data with that leaf location
// (allocation info and the callsites).
std::map<uint64_t, std::set<const AllocationInfo *>> LocHashToAllocInfo;
- // A thin wrapper around ArrayRef<Frame> to facilitate std::set<CallStackRef>.
- struct CallStackRef : public ArrayRef<Frame> {
- CallStackRef(ArrayRef<Frame> CS, unsigned Pos)
- : ArrayRef<Frame>(CS.drop_front(Pos)) {}
- // std::set requires std::less.
- bool operator<(const CallStackRef &R) const {
- const CallStackRef &L = *this;
- return std::make_pair(L.data(), L.size()) <
- std::make_pair(R.data(), R.size());
+ // A hash function for std::unordered_set<ArrayRef<Frame>> to work.
+ struct CallStackHash {
+ size_t operator()(ArrayRef<Frame> CS) const {
+ return computeFullStackId(CS);
}
};
// For the callsites we need to record slices of the frame array (see comments
// below where the map entries are added).
- std::map<uint64_t, std::set<CallStackRef>> LocHashToCallSites;
+ std::map<uint64_t, std::unordered_set<ArrayRef<Frame>, CallStackHash>>
+ LocHashToCallSites;
for (auto &AI : MemProfRec->AllocSites) {
NumOfMemProfAllocContextProfiles++;
// Associate the allocation info with the leaf frame. The later matching
@@ -998,7 +993,8 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
unsigned Idx = 0;
for (auto &StackFrame : CS) {
uint64_t StackId = computeStackId(StackFrame);
- LocHashToCallSites[StackId].emplace(CS, Idx++);
+ LocHashToCallSites[StackId].insert(
+ ArrayRef<Frame>(CS).drop_front(Idx++));
ProfileHasColumns |= StackFrame.Column;
// Once we find this function, we can stop recording.
if (StackFrame.Function == FuncGUID)
@@ -1038,7 +1034,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
// and another callsite).
std::map<uint64_t, std::set<const AllocationInfo *>>::iterator
AllocInfoIter;
- std::map<uint64_t, std::set<CallStackRef>>::iterator CallSitesIter;
+ decltype(LocHashToCallSites)::iterator CallSitesIter;
for (const DILocation *DIL = I.getDebugLoc(); DIL != nullptr;
DIL = DIL->getInlinedAt()) {
// Use C++ linkage name if possible. Need to compile with
>From a790e98d514a7f585dc0f237f5e956b0d6407a58 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Fri, 13 Dec 2024 21:36:45 -0800
Subject: [PATCH 3/3] Address comments.
---
llvm/lib/Transforms/Instrumentation/MemProfiler.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 0b310d9c8a01bf..c980869a1c0d82 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -993,8 +993,7 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
unsigned Idx = 0;
for (auto &StackFrame : CS) {
uint64_t StackId = computeStackId(StackFrame);
- LocHashToCallSites[StackId].insert(
- ArrayRef<Frame>(CS).drop_front(Idx++));
+ LocHashToCallSites[StackId].insert(ArrayRef<Frame>(CS).drop_front(Idx++));
ProfileHasColumns |= StackFrame.Column;
// Once we find this function, we can stop recording.
if (StackFrame.Function == FuncGUID)
More information about the llvm-commits
mailing list