Skip to content

Commit 10d054e

Browse files
kazutakahiratakazutakahirata
authored
[memprof] Introduce IndexedCallstackIdConveter (NFC) (#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.
1 parent e3b571e commit 10d054e

File tree

3 files changed

+45
-48
lines changed

3 files changed

+45
-48
lines changed

‎llvm/include/llvm/ProfileData/MemProf.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,27 @@ struct IndexedMemProfData {
10301030
CallStackId hashCallStack(ArrayRef<FrameId> CS) const;
10311031
};
10321032

1033+
// A convenience wrapper around FrameIdConverter and CallStackIdConverter for
1034+
// tests.
1035+
struct IndexedCallstackIdConveter {
1036+
IndexedCallstackIdConveter() = delete;
1037+
IndexedCallstackIdConveter(IndexedMemProfData &MemProfData)
1038+
: FrameIdConv(MemProfData.Frames),
1039+
CSIdConv(MemProfData.CallStacks, FrameIdConv) {}
1040+
1041+
// Delete the copy constructor and copy assignment operator to avoid a
1042+
// situation where a copy of IndexedCallStackIdConverter gets an error in
1043+
// LastUnmappedId while the original instance doesn't.
1044+
IndexedCallstackIdConveter(const IndexedCallstackIdConveter &) = delete;
1045+
IndexedCallstackIdConveter &
1046+
operator=(const IndexedCallstackIdConveter &) = delete;
1047+
1048+
std::vector<Frame> operator()(CallStackId CSId) { return CSIdConv(CSId); }
1049+
1050+
FrameIdConverter<decltype(IndexedMemProfData::Frames)> FrameIdConv;
1051+
CallStackIdConverter<decltype(IndexedMemProfData::CallStacks)> CSIdConv;
1052+
};
1053+
10331054
struct FrameStat {
10341055
// The number of occurrences of a given FrameId.
10351056
uint64_t Count = 0;

‎llvm/unittests/ProfileData/InstrProfTest.cpp‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -457,17 +457,14 @@ TEST_F(InstrProfTest, test_memprof_v2_full_schema) {
457457
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
458458
const memprof::MemProfRecord &Record = RecordOr.get();
459459

460-
memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
461-
MemProfData.Frames);
462-
memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
463-
MemProfData.CallStacks, FrameIdConv);
460+
memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
464461

465462
const ::llvm::memprof::MemProfRecord WantRecord =
466463
IndexedMR.toMemProfRecord(CSIdConv);
467-
ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt)
468-
<< "could not map frame id: " << *FrameIdConv.LastUnmappedId;
469-
ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt)
470-
<< "could not map call stack id: " << *CSIdConv.LastUnmappedId;
464+
ASSERT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt)
465+
<< "could not map frame id: " << *CSIdConv.FrameIdConv.LastUnmappedId;
466+
ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt)
467+
<< "could not map call stack id: " << *CSIdConv.CSIdConv.LastUnmappedId;
471468
EXPECT_THAT(WantRecord, EqualsRecord(Record));
472469
}
473470

@@ -494,17 +491,14 @@ TEST_F(InstrProfTest, test_memprof_v2_partial_schema) {
494491
ASSERT_THAT_ERROR(RecordOr.takeError(), Succeeded());
495492
const memprof::MemProfRecord &Record = RecordOr.get();
496493

497-
memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
498-
MemProfData.Frames);
499-
memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
500-
MemProfData.CallStacks, FrameIdConv);
494+
memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
501495

502496
const ::llvm::memprof::MemProfRecord WantRecord =
503497
IndexedMR.toMemProfRecord(CSIdConv);
504-
ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt)
505-
<< "could not map frame id: " << *FrameIdConv.LastUnmappedId;
506-
ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt)
507-
<< "could not map call stack id: " << *CSIdConv.LastUnmappedId;
498+
ASSERT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt)
499+
<< "could not map frame id: " << *CSIdConv.FrameIdConv.LastUnmappedId;
500+
ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt)
501+
<< "could not map call stack id: " << *CSIdConv.CSIdConv.LastUnmappedId;
508502
EXPECT_THAT(WantRecord, EqualsRecord(Record));
509503
}
510504

@@ -615,10 +609,7 @@ TEST_F(InstrProfTest, test_memprof_merge) {
615609

616610
std::optional<memprof::FrameId> LastUnmappedFrameId;
617611

618-
memprof::FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
619-
MemProfData.Frames);
620-
memprof::CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
621-
MemProfData.CallStacks, FrameIdConv);
612+
memprof::IndexedCallstackIdConveter CSIdConv(MemProfData);
622613

623614
const ::llvm::memprof::MemProfRecord WantRecord =
624615
IndexedMR.toMemProfRecord(CSIdConv);

‎llvm/unittests/ProfileData/MemProfTest.cpp‎

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -501,16 +501,13 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
501501
IndexedRecord.CallSiteIds.push_back(CS3Id);
502502
IndexedRecord.CallSiteIds.push_back(CS4Id);
503503

504-
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
505-
MemProfData.Frames);
506-
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
507-
MemProfData.CallStacks, FrameIdConv);
504+
IndexedCallstackIdConveter CSIdConv(MemProfData);
508505

509506
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
510507

511508
// Make sure that all lookups are successful.
512-
ASSERT_EQ(FrameIdConv.LastUnmappedId, std::nullopt);
513-
ASSERT_EQ(CSIdConv.LastUnmappedId, std::nullopt);
509+
ASSERT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt);
510+
ASSERT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt);
514511

515512
// Verify the contents of Record.
516513
ASSERT_THAT(Record.AllocSites, SizeIs(2));
@@ -540,17 +537,14 @@ TEST(MemProf, MissingCallStackId) {
540537

541538
// Create empty maps.
542539
IndexedMemProfData MemProfData;
543-
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
544-
MemProfData.Frames);
545-
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
546-
MemProfData.CallStacks, FrameIdConv);
540+
IndexedCallstackIdConveter CSIdConv(MemProfData);
547541

548542
// We are only interested in errors, not the return value.
549543
(void)IndexedMR.toMemProfRecord(CSIdConv);
550544

551-
ASSERT_TRUE(CSIdConv.LastUnmappedId.has_value());
552-
EXPECT_EQ(*CSIdConv.LastUnmappedId, 0xdeadbeefU);
553-
EXPECT_EQ(FrameIdConv.LastUnmappedId, std::nullopt);
545+
ASSERT_TRUE(CSIdConv.CSIdConv.LastUnmappedId.has_value());
546+
EXPECT_EQ(*CSIdConv.CSIdConv.LastUnmappedId, 0xdeadbeefU);
547+
EXPECT_EQ(CSIdConv.FrameIdConv.LastUnmappedId, std::nullopt);
554548
}
555549

556550
TEST(MemProf, MissingFrameId) {
@@ -561,17 +555,14 @@ TEST(MemProf, MissingFrameId) {
561555
IndexedMemProfRecord IndexedMR;
562556
IndexedMR.AllocSites.emplace_back(CSId, makePartialMIB(), getHotColdSchema());
563557

564-
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
565-
MemProfData.Frames);
566-
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
567-
MemProfData.CallStacks, FrameIdConv);
558+
IndexedCallstackIdConveter CSIdConv(MemProfData);
568559

569560
// We are only interested in errors, not the return value.
570561
(void)IndexedMR.toMemProfRecord(CSIdConv);
571562

572-
EXPECT_EQ(CSIdConv.LastUnmappedId, std::nullopt);
573-
ASSERT_TRUE(FrameIdConv.LastUnmappedId.has_value());
574-
EXPECT_EQ(*FrameIdConv.LastUnmappedId, 3U);
563+
EXPECT_EQ(CSIdConv.CSIdConv.LastUnmappedId, std::nullopt);
564+
ASSERT_TRUE(CSIdConv.FrameIdConv.LastUnmappedId.has_value());
565+
EXPECT_EQ(*CSIdConv.FrameIdConv.LastUnmappedId, 3U);
575566
}
576567

577568
// Verify CallStackRadixTreeBuilder can handle empty inputs.
@@ -714,10 +705,7 @@ TEST(MemProf, YAMLParser) {
714705
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
715706
EXPECT_EQ(GUID, 0xdeadbeef12345678ULL);
716707

717-
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
718-
MemProfData.Frames);
719-
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
720-
MemProfData.CallStacks, FrameIdConv);
708+
IndexedCallstackIdConveter CSIdConv(MemProfData);
721709
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
722710

723711
ASSERT_THAT(Record.AllocSites, SizeIs(2));
@@ -760,10 +748,7 @@ TEST(MemProf, YAMLParserGUID) {
760748
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
761749
EXPECT_EQ(GUID, IndexedMemProfRecord::getGUID("_Z3fooi"));
762750

763-
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
764-
MemProfData.Frames);
765-
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
766-
MemProfData.CallStacks, FrameIdConv);
751+
IndexedCallstackIdConveter CSIdConv(MemProfData);
767752
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
768753

769754
ASSERT_THAT(Record.AllocSites, SizeIs(1));

0 commit comments

Comments
 (0)