[llvm-branch-commits] [llvm] 884d89a - Revert "[memprof] Introduce FrameIdConverter and CallStackIdConverter (#90307)"
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Apr 27 00:09:11 PDT 2024
Author: Vitaly Buka
Date: 2024-04-27T00:09:08-07:00
New Revision: 884d89aab141a3016d8d5b42ed163e649a969ad4
URL: https://github.com/llvm/llvm-project/commit/884d89aab141a3016d8d5b42ed163e649a969ad4
DIFF: https://github.com/llvm/llvm-project/commit/884d89aab141a3016d8d5b42ed163e649a969ad4.diff
LOG: Revert "[memprof] Introduce FrameIdConverter and CallStackIdConverter (#90307)"
This reverts commit e04df693bf5b38099ef1d7ab8e6ce6a1469597e2.
Added:
Modified:
llvm/include/llvm/ProfileData/MemProf.h
llvm/include/llvm/ProfileData/MemProfReader.h
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
llvm/unittests/ProfileData/MemProfTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 8b00faf2a219df..d378c3696f8d0b 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -737,64 +737,6 @@ class CallStackLookupTrait {
// Compute a CallStackId for a given call stack.
CallStackId hashCallStack(ArrayRef<FrameId> CS);
-namespace detail {
-// "Dereference" the iterator from DenseMap or OnDiskChainedHashTable. We have
-// to do so in one of two
diff erent ways depending on the type of the hash
-// table.
-template <typename value_type, typename IterTy>
-value_type DerefIterator(IterTy Iter) {
- using deref_type = llvm::remove_cvref_t<decltype(*Iter)>;
- if constexpr (std::is_same_v<deref_type, value_type>)
- return *Iter;
- else
- return Iter->second;
-}
-} // namespace detail
-
-// A function object that returns a frame for a given FrameId.
-template <typename MapTy> struct FrameIdConverter {
- std::optional<FrameId> LastUnmappedId;
- MapTy ⤅
-
- FrameIdConverter() = delete;
- FrameIdConverter(MapTy &Map) : Map(Map) {}
-
- Frame operator()(FrameId Id) {
- auto Iter = Map.find(Id);
- if (Iter == Map.end()) {
- LastUnmappedId = Id;
- return Frame(0, 0, 0, false);
- }
- return detail::DerefIterator<Frame>(Iter);
- }
-};
-
-// A function object that returns a call stack for a given CallStackId.
-template <typename MapTy> struct CallStackIdConverter {
- std::optional<CallStackId> LastUnmappedId;
- MapTy ⤅
- std::function<Frame(FrameId)> FrameIdToFrame;
-
- CallStackIdConverter() = delete;
- CallStackIdConverter(MapTy &Map, std::function<Frame(FrameId)> FrameIdToFrame)
- : Map(Map), FrameIdToFrame(FrameIdToFrame) {}
-
- llvm::SmallVector<Frame> operator()(CallStackId CSId) {
- llvm::SmallVector<Frame> Frames;
- auto CSIter = Map.find(CSId);
- if (CSIter == Map.end()) {
- LastUnmappedId = CSId;
- } else {
- llvm::SmallVector<FrameId> CS =
- detail::DerefIterator<llvm::SmallVector<FrameId>>(CSIter);
- Frames.reserve(CS.size());
- for (FrameId Id : CS)
- Frames.push_back(FrameIdToFrame(Id));
- }
- return Frames;
- }
-};
-
// Verify that each CallStackId is computed with hashCallStack. This function
// is intended to help transition from CallStack to CSId in
// IndexedAllocationInfo.
diff --git a/llvm/include/llvm/ProfileData/MemProfReader.h b/llvm/include/llvm/ProfileData/MemProfReader.h
index b42e4f59777409..444c58e8bdc8bc 100644
--- a/llvm/include/llvm/ProfileData/MemProfReader.h
+++ b/llvm/include/llvm/ProfileData/MemProfReader.h
@@ -76,16 +76,20 @@ class MemProfReader {
Callback =
std::bind(&MemProfReader::idToFrame, this, std::placeholders::_1);
- memprof::CallStackIdConverter<decltype(CSIdToCallStack)> CSIdConv(
- CSIdToCallStack, Callback);
+ auto CallStackCallback = [&](CallStackId CSId) {
+ llvm::SmallVector<Frame> CallStack;
+ auto Iter = CSIdToCallStack.find(CSId);
+ assert(Iter != CSIdToCallStack.end());
+ for (FrameId Id : Iter->second)
+ CallStack.push_back(Callback(Id));
+ return CallStack;
+ };
const IndexedMemProfRecord &IndexedRecord = Iter->second;
GuidRecord = {
Iter->first,
- IndexedRecord.toMemProfRecord(CSIdConv),
+ IndexedRecord.toMemProfRecord(CallStackCallback),
};
- if (CSIdConv.LastUnmappedId)
- return make_error<InstrProfError>(instrprof_error::hash_mismatch);
Iter++;
return Error::success();
}
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 440be2f255d392..cefb6af12d0021 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1520,35 +1520,53 @@ IndexedMemProfReader::getMemProfRecord(const uint64_t FuncNameHash) const {
// Setup a callback to convert from frame ids to frame using the on-disk
// FrameData hash table.
- memprof::FrameIdConverter<MemProfFrameHashTable> FrameIdConv(
- *MemProfFrameTable.get());
+ std::optional<memprof::FrameId> LastUnmappedFrameId;
+ auto IdToFrameCallback = [&](const memprof::FrameId Id) {
+ auto FrIter = MemProfFrameTable->find(Id);
+ if (FrIter == MemProfFrameTable->end()) {
+ LastUnmappedFrameId = Id;
+ return memprof::Frame(0, 0, 0, false);
+ }
+ return *FrIter;
+ };
// Setup a callback to convert call stack ids to call stacks using the on-disk
// hash table.
- memprof::CallStackIdConverter<MemProfCallStackHashTable> CSIdConv(
- *MemProfCallStackTable.get(), FrameIdConv);
+ std::optional<memprof::CallStackId> LastUnmappedCSId;
+ auto CSIdToCallStackCallback = [&](memprof::CallStackId CSId) {
+ llvm::SmallVector<memprof::Frame> Frames;
+ auto CSIter = MemProfCallStackTable->find(CSId);
+ if (CSIter == MemProfCallStackTable->end()) {
+ LastUnmappedCSId = CSId;
+ } else {
+ const llvm::SmallVector<memprof::FrameId> &CS = *CSIter;
+ Frames.reserve(CS.size());
+ for (memprof::FrameId Id : CS)
+ Frames.push_back(IdToFrameCallback(Id));
+ }
+ return Frames;
+ };
const memprof::IndexedMemProfRecord IndexedRecord = *Iter;
memprof::MemProfRecord Record;
if (MemProfCallStackTable)
- Record = IndexedRecord.toMemProfRecord(CSIdConv);
+ Record = IndexedRecord.toMemProfRecord(CSIdToCallStackCallback);
else
- Record = memprof::MemProfRecord(IndexedRecord, FrameIdConv);
+ Record = memprof::MemProfRecord(IndexedRecord, IdToFrameCallback);
// Check that all frame ids were successfully converted to frames.
- if (FrameIdConv.LastUnmappedId) {
- return make_error<InstrProfError>(
- instrprof_error::hash_mismatch,
- "memprof frame not found for frame id " +
- Twine(*FrameIdConv.LastUnmappedId));
+ if (LastUnmappedFrameId) {
+ return make_error<InstrProfError>(instrprof_error::hash_mismatch,
+ "memprof frame not found for frame id " +
+ Twine(*LastUnmappedFrameId));
}
// Check that all call stack ids were successfully converted to call stacks.
- if (CSIdConv.LastUnmappedId) {
+ if (LastUnmappedCSId) {
return make_error<InstrProfError>(
instrprof_error::hash_mismatch,
"memprof call stack not found for call stack id " +
- Twine(*CSIdConv.LastUnmappedId));
+ Twine(*LastUnmappedCSId));
}
return Record;
}
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index acc633de11b6bd..edc427dcbc4540 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -495,6 +495,44 @@ TEST_F(InstrProfTest, test_memprof_v0) {
EXPECT_THAT(WantRecord, EqualsRecord(Record));
}
+struct CallStackIdConverter {
+ std::optional<memprof::FrameId> LastUnmappedFrameId;
+ std::optional<memprof::CallStackId> LastUnmappedCSId;
+
+ const FrameIdMapTy &IdToFrameMap;
+ const CallStackIdMapTy &CSIdToCallStackMap;
+
+ CallStackIdConverter() = delete;
+ CallStackIdConverter(const FrameIdMapTy &IdToFrameMap,
+ const CallStackIdMapTy &CSIdToCallStackMap)
+ : IdToFrameMap(IdToFrameMap), CSIdToCallStackMap(CSIdToCallStackMap) {}
+
+ llvm::SmallVector<memprof::Frame>
+ operator()(::llvm::memprof::CallStackId CSId) {
+ auto IdToFrameCallback = [&](const memprof::FrameId Id) {
+ auto Iter = IdToFrameMap.find(Id);
+ if (Iter == IdToFrameMap.end()) {
+ LastUnmappedFrameId = Id;
+ return memprof::Frame(0, 0, 0, false);
+ }
+ return Iter->second;
+ };
+
+ llvm::SmallVector<memprof::Frame> Frames;
+ auto CSIter = CSIdToCallStackMap.find(CSId);
+ if (CSIter == CSIdToCallStackMap.end()) {
+ LastUnmappedCSId = CSId;
+ } else {
+ const ::llvm::SmallVector<::llvm::memprof::FrameId> &CS =
+ CSIter->getSecond();
+ Frames.reserve(CS.size());
+ for (::llvm::memprof::FrameId Id : CS)
+ Frames.push_back(IdToFrameCallback(Id));
+ }
+ return Frames;
+ }
+};
+
TEST_F(InstrProfTest, test_memprof_v2_full_schema) {
const MemInfoBlock MIB = makeFullMIB();
@@ -524,16 +562,14 @@ TEST_F(InstrProfTest, test_memprof_v2_full_schema) {
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
const memprof::MemProfRecord &Record = RecordOr.get();
- memprof::FrameIdConverter<decltype(IdToFrameMap)> FrameIdConv(IdToFrameMap);
- memprof::CallStackIdConverter<decltype(CSIdToCallStackMap)> CSIdConv(
- CSIdToCallStackMap, FrameIdConv);
+ CallStackIdConverter CSIdConv(IdToFrameMap, CSIdToCallStackMap);
const ::llvm::memprof::MemProfRecord WantRecord =
IndexedMR.toMemProfRecord(CSIdConv);
- ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt)
- << "could not map frame id: " << *FrameIdConv.LastUnmappedId;
- ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt)
- << "could not map call stack id: " << *CSIdConv.LastUnmappedId;
+ ASSERT_EQ(CSIdConv.LastUnmappedFrameId, std::nullopt)
+ << "could not map frame id: " << *CSIdConv.LastUnmappedFrameId;
+ ASSERT_EQ(CSIdConv.LastUnmappedCSId, std::nullopt)
+ << "could not map call stack id: " << *CSIdConv.LastUnmappedCSId;
EXPECT_THAT(WantRecord, EqualsRecord(Record));
}
@@ -566,16 +602,14 @@ TEST_F(InstrProfTest, test_memprof_v2_partial_schema) {
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
const memprof::MemProfRecord &Record = RecordOr.get();
- memprof::FrameIdConverter<decltype(IdToFrameMap)> FrameIdConv(IdToFrameMap);
- memprof::CallStackIdConverter<decltype(CSIdToCallStackMap)> CSIdConv(
- CSIdToCallStackMap, FrameIdConv);
+ CallStackIdConverter CSIdConv(IdToFrameMap, CSIdToCallStackMap);
const ::llvm::memprof::MemProfRecord WantRecord =
IndexedMR.toMemProfRecord(CSIdConv);
- ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt)
- << "could not map frame id: " << *FrameIdConv.LastUnmappedId;
- ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt)
- << "could not map call stack id: " << *CSIdConv.LastUnmappedId;
+ ASSERT_EQ(CSIdConv.LastUnmappedFrameId, std::nullopt)
+ << "could not map frame id: " << *CSIdConv.LastUnmappedFrameId;
+ ASSERT_EQ(CSIdConv.LastUnmappedCSId, std::nullopt)
+ << "could not map call stack id: " << *CSIdConv.LastUnmappedCSId;
EXPECT_THAT(WantRecord, EqualsRecord(Record));
}
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index d031049cea14bf..98dacd3511e1d8 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -502,15 +502,37 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
IndexedRecord.CallSiteIds.push_back(llvm::memprof::hashCallStack(CS3));
IndexedRecord.CallSiteIds.push_back(llvm::memprof::hashCallStack(CS4));
- llvm::memprof::FrameIdConverter<decltype(FrameIdMap)> FrameIdConv(FrameIdMap);
- llvm::memprof::CallStackIdConverter<decltype(CallStackIdMap)> CSIdConv(
- CallStackIdMap, FrameIdConv);
-
- MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
+ bool CSIdMissing = false;
+ bool FrameIdMissing = false;
+
+ auto Callback = [&](CallStackId CSId) -> llvm::SmallVector<Frame> {
+ llvm::SmallVector<Frame> CallStack;
+ llvm::SmallVector<FrameId> FrameIds;
+
+ auto Iter = CallStackIdMap.find(CSId);
+ if (Iter == CallStackIdMap.end())
+ CSIdMissing = true;
+ else
+ FrameIds = Iter->second;
+
+ for (FrameId Id : FrameIds) {
+ Frame F(0, 0, 0, false);
+ auto Iter = FrameIdMap.find(Id);
+ if (Iter == FrameIdMap.end())
+ FrameIdMissing = true;
+ else
+ F = Iter->second;
+ CallStack.push_back(F);
+ }
+
+ return CallStack;
+ };
+
+ MemProfRecord Record = IndexedRecord.toMemProfRecord(Callback);
// Make sure that all lookups are successful.
- ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt);
- ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt);
+ ASSERT_FALSE(CSIdMissing);
+ ASSERT_FALSE(FrameIdMissing);
// Verify the contents of Record.
ASSERT_THAT(Record.AllocSites, SizeIs(2));
More information about the llvm-branch-commits
mailing list