[llvm] [memprof] Introduce IndexedCallstackIdConveter (NFC) (PR #120540)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 01:17:05 PST 2024
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/120540
This patch introduces IndexedCallstackIdConveter as a convenience
wrapper around FrameIdConverter and CallStackIdConverter just for
tests.
With the new wrapper, we get to replace idioms like:
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
MemProfData.Frames);
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
MemProfData.CallStacks, FrameIdConv);
with:
IndexedCallstackIdConveter CSIdConv(MemProfData);
Unfortunately, this exact pattern occurs in tests only; the
combinations of the frame ID converter and call stack ID converter are
diverse in production code.
>From 0a6f4a27251d71d65eebea95dad3c9ce628bde32 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Wed, 18 Dec 2024 17:44:36 -0800
Subject: [PATCH] [memprof] Introduce IndexedCallstackIdConveter (NFC)
This patch introduces IndexedCallstackIdConveter as a convenience
wrapper around FrameIdConverter and CallStackIdConverter just for
tests.
With the new wrapper, we get to replace idioms like:
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
MemProfData.Frames);
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
MemProfData.CallStacks, FrameIdConv);
with:
IndexedCallstackIdConveter CSIdConv(MemProfData);
Unfortunately, this exact pattern occurs in tests only; the
combinations of the frame ID converter and call stack ID converter are
diverse in production code.
---
llvm/include/llvm/ProfileData/MemProf.h | 21 ++++++++++
llvm/unittests/ProfileData/InstrProfTest.cpp | 31 ++++++---------
llvm/unittests/ProfileData/MemProfTest.cpp | 41 +++++++-------------
3 files changed, 45 insertions(+), 48 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 883e24d7186152..da0cb47508e32b 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -1030,6 +1030,27 @@ struct IndexedMemProfData {
CallStackId hashCallStack(ArrayRef<FrameId> CS) const;
};
+// A convenience wrapper around FrameIdConverter and CallStackIdConverter for
+// tests.
+struct IndexedCallstackIdConveter {
+ IndexedCallstackIdConveter() = delete;
+ IndexedCallstackIdConveter(IndexedMemProfData &MemProfData)
+ : FrameIdConv(MemProfData.Frames),
+ CSIdConv(MemProfData.CallStacks, FrameIdConv) {}
+
+ // Delete the copy constructor and copy assignment operator to avoid a
+ // situation where a copy of IndexedCallStackIdConverter gets an error in
+ // LastUnmappedId while the original instance doesn't.
+ IndexedCallstackIdConveter(const IndexedCallstackIdConveter &) = delete;
+ IndexedCallstackIdConveter &
+ operator=(const IndexedCallstackIdConveter &) = delete;
+
+ std::vector<Frame> operator()(CallStackId CSId) { return CSIdConv(CSId); }
+
+ FrameIdConverter<decltype(IndexedMemProfData::Frames)> FrameIdConv;
+ CallStackIdConverter<decltype(IndexedMemProfData::CallStacks)> CSIdConv;
+};
+
struct FrameStat {
// The number of occurrences of a given FrameId.
uint64_t Count = 0;
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
index ac872d8235622b..adcd4d2d11e0f1 100644
--- a/llvm/unittests/ProfileData/InstrProfTest.cpp
+++ b/llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -457,17 +457,14 @@ TEST_F(InstrProfTest, test_memprof_v2_full_schema) {
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
const memprof::MemProfRecord &Record = RecordOr.get();
- memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
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.FrameIdConv.LastUnmappedId, std::nullopt)
+ << "could not map frame id: " << *CSIdConv.FrameIdConv.LastUnmappedId;
+ ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt)
+ << "could not map call stack id: " << *CSIdConv.CSIdConv.LastUnmappedId;
EXPECT_THAT(WantRecord, EqualsRecord(Record));
}
@@ -494,17 +491,14 @@ TEST_F(InstrProfTest, test_memprof_v2_partial_schema) {
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
const memprof::MemProfRecord &Record = RecordOr.get();
- memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
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.FrameIdConv.LastUnmappedId, std::nullopt)
+ << "could not map frame id: " << *CSIdConv.FrameIdConv.LastUnmappedId;
+ ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt)
+ << "could not map call stack id: " << *CSIdConv.CSIdConv.LastUnmappedId;
EXPECT_THAT(WantRecord, EqualsRecord(Record));
}
@@ -615,10 +609,7 @@ TEST_F(InstrProfTest, test_memprof_merge) {
std::optional<memprof::FrameId> LastUnmappedFrameId;
- memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
const ::llvm::memprof::MemProfRecord WantRecord =
IndexedMR.toMemProfRecord(CSIdConv);
diff --git a/llvm/unittests/ProfileData/MemProfTest.cpp b/llvm/unittests/ProfileData/MemProfTest.cpp
index 2eb85d5b2f5878..1fe9af521d884c 100644
--- a/llvm/unittests/ProfileData/MemProfTest.cpp
+++ b/llvm/unittests/ProfileData/MemProfTest.cpp
@@ -501,16 +501,13 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
IndexedRecord.CallSiteIds.push_back(CS3Id);
IndexedRecord.CallSiteIds.push_back(CS4Id);
- FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ IndexedCallstackIdConveter CSIdConv(MemProfData);
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
// Make sure that all lookups are successful.
- ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt);
- ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt);
+ ASSERT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt);
+ ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt);
// Verify the contents of Record.
ASSERT_THAT(Record.AllocSites, SizeIs(2));
@@ -540,17 +537,14 @@ TEST(MemProf, MissingCallStackId) {
// Create empty maps.
IndexedMemProfData MemProfData;
- FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ IndexedCallstackIdConveter CSIdConv(MemProfData);
// We are only interested in errors, not the return value.
(void)IndexedMR.toMemProfRecord(CSIdConv);
- ASSERT_TRUE(CSIdConv.LastUnmappedId.has_value());
- EXPECT_EQ(*CSIdConv.LastUnmappedId, 0xdeadbeefU);
- EXPECT_EQ(FrameIdConv.LastUnmappedId, std::nullopt);
+ ASSERT_TRUE(CSIdConv.CSIdConv.LastUnmappedId.has_value());
+ EXPECT_EQ(*CSIdConv.CSIdConv.LastUnmappedId, 0xdeadbeefU);
+ EXPECT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt);
}
TEST(MemProf, MissingFrameId) {
@@ -561,17 +555,14 @@ TEST(MemProf, MissingFrameId) {
IndexedMemProfRecord IndexedMR;
IndexedMR.AllocSites.emplace_back(CSId, makePartialMIB(), getHotColdSchema());
- FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ IndexedCallstackIdConveter CSIdConv(MemProfData);
// We are only interested in errors, not the return value.
(void)IndexedMR.toMemProfRecord(CSIdConv);
- EXPECT_EQ(CSIdConv.LastUnmappedId, std::nullopt);
- ASSERT_TRUE(FrameIdConv.LastUnmappedId.has_value());
- EXPECT_EQ(*FrameIdConv.LastUnmappedId, 3U);
+ EXPECT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt);
+ ASSERT_TRUE(CSIdConv.FrameIdConv.LastUnmappedId.has_value());
+ EXPECT_EQ(*CSIdConv.FrameIdConv.LastUnmappedId, 3U);
}
// Verify CallStackRadixTreeBuilder can handle empty inputs.
@@ -714,10 +705,7 @@ TEST(MemProf, YAMLParser) {
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
EXPECT_EQ(GUID, 0xdeadbeef12345678ULL);
- FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ IndexedCallstackIdConveter CSIdConv(MemProfData);
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
ASSERT_THAT(Record.AllocSites, SizeIs(2));
@@ -760,10 +748,7 @@ TEST(MemProf, YAMLParserGUID) {
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
EXPECT_EQ(GUID, IndexedMemProfRecord::getGUID("_Z3fooi"));
- FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
- MemProfData.Frames);
- CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
- MemProfData.CallStacks, FrameIdConv);
+ IndexedCallstackIdConveter CSIdConv(MemProfData);
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
ASSERT_THAT(Record.AllocSites, SizeIs(1));
More information about the llvm-commits
mailing list