Skip to content

Commit 7f2fb80

Browse files
[memprof] Don't use Frame::hash or hashCallStacks in unit test (#119984)
This patch checks the result of YAML parsing at the level of MemProfRecord instead of IndexedMemProfRecord, thereby avoiding use of Frame::hash and hashCallStacks. This makes sense because we ultimately care about consumers like MemProfiler.cpp obtaining MemProfRecord correctly; IndexedMemProfData and hash values are just intermediaries. Once this patch lands, we call Frame::hash and hashCallStacks only when adding Frames or call stacks to their respective data structures. In other words, the hash functions are pretty much business internal to IndexedMemProfRecord.
1 parent 48c20e7 commit 7f2fb80

File tree

1 file changed

+30
-47
lines changed

1 file changed

+30
-47
lines changed

llvm/unittests/ProfileData/MemProfTest.cpp

Lines changed: 30 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -709,47 +709,33 @@ TEST(MemProf, YAMLParser) {
709709
YAMLReader.parse(YAMLData);
710710
IndexedMemProfData MemProfData = YAMLReader.takeMemProfData();
711711

712-
Frame F1(0x100, 11, 10, true);
713-
Frame F2(0x200, 22, 20, false);
714-
Frame F3(0x300, 33, 30, false);
715-
Frame F4(0x400, 44, 40, true);
716-
Frame F5(0x500, 55, 50, true);
717-
Frame F6(0x600, 66, 60, false);
718-
Frame F7(0x700, 77, 70, true);
719-
Frame F8(0x800, 88, 80, false);
720-
721-
llvm::SmallVector<FrameId> CS1 = {F1.hash(), F2.hash()};
722-
llvm::SmallVector<FrameId> CS2 = {F3.hash(), F4.hash()};
723-
llvm::SmallVector<FrameId> CS3 = {F5.hash(), F6.hash()};
724-
llvm::SmallVector<FrameId> CS4 = {F7.hash(), F8.hash()};
725-
726-
// Verify the entire contents of MemProfData.Frames.
727-
EXPECT_THAT(MemProfData.Frames,
728-
UnorderedElementsAre(Pair(F1.hash(), F1), Pair(F2.hash(), F2),
729-
Pair(F3.hash(), F3), Pair(F4.hash(), F4),
730-
Pair(F5.hash(), F5), Pair(F6.hash(), F6),
731-
Pair(F7.hash(), F7), Pair(F8.hash(), F8)));
732-
733-
// Verify the entire contents of MemProfData.Frames.
734-
EXPECT_THAT(MemProfData.CallStacks,
735-
UnorderedElementsAre(Pair(hashCallStack(CS1), CS1),
736-
Pair(hashCallStack(CS2), CS2),
737-
Pair(hashCallStack(CS3), CS3),
738-
Pair(hashCallStack(CS4), CS4)));
739-
740712
// Verify the entire contents of MemProfData.Records.
741713
ASSERT_THAT(MemProfData.Records, SizeIs(1));
742-
const auto &[GUID, Record] = MemProfData.Records.front();
714+
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
743715
EXPECT_EQ(GUID, 0xdeadbeef12345678ULL);
716+
717+
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
718+
MemProfData.Frames);
719+
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
720+
MemProfData.CallStacks, FrameIdConv);
721+
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
722+
744723
ASSERT_THAT(Record.AllocSites, SizeIs(2));
745-
EXPECT_EQ(Record.AllocSites[0].CSId, hashCallStack(CS1));
724+
EXPECT_THAT(
725+
Record.AllocSites[0].CallStack,
726+
ElementsAre(Frame(0x100, 11, 10, true), Frame(0x200, 22, 20, false)));
746727
EXPECT_EQ(Record.AllocSites[0].Info.getAllocCount(), 777U);
747728
EXPECT_EQ(Record.AllocSites[0].Info.getTotalSize(), 888U);
748-
EXPECT_EQ(Record.AllocSites[1].CSId, hashCallStack(CS2));
729+
EXPECT_THAT(
730+
Record.AllocSites[1].CallStack,
731+
ElementsAre(Frame(0x300, 33, 30, false), Frame(0x400, 44, 40, true)));
749732
EXPECT_EQ(Record.AllocSites[1].Info.getAllocCount(), 666U);
750733
EXPECT_EQ(Record.AllocSites[1].Info.getTotalSize(), 555U);
751-
EXPECT_THAT(Record.CallSiteIds,
752-
ElementsAre(hashCallStack(CS3), hashCallStack(CS4)));
734+
EXPECT_THAT(Record.CallSites,
735+
ElementsAre(ElementsAre(Frame(0x500, 55, 50, true),
736+
Frame(0x600, 66, 60, false)),
737+
ElementsAre(Frame(0x700, 77, 70, true),
738+
Frame(0x800, 88, 80, false))));
753739
}
754740

755741
// Verify that the YAML parser accepts a GUID expressed as a function name.
@@ -769,24 +755,21 @@ TEST(MemProf, YAMLParserGUID) {
769755
YAMLReader.parse(YAMLData);
770756
IndexedMemProfData MemProfData = YAMLReader.takeMemProfData();
771757

772-
Frame F1(0x100, 11, 10, true);
773-
774-
llvm::SmallVector<FrameId> CS1 = {F1.hash()};
775-
776-
// Verify the entire contents of MemProfData.Frames.
777-
EXPECT_THAT(MemProfData.Frames, UnorderedElementsAre(Pair(F1.hash(), F1)));
778-
779-
// Verify the entire contents of MemProfData.Frames.
780-
EXPECT_THAT(MemProfData.CallStacks,
781-
UnorderedElementsAre(Pair(hashCallStack(CS1), CS1)));
782-
783758
// Verify the entire contents of MemProfData.Records.
784759
ASSERT_THAT(MemProfData.Records, SizeIs(1));
785-
const auto &[GUID, Record] = MemProfData.Records.front();
760+
const auto &[GUID, IndexedRecord] = MemProfData.Records.front();
786761
EXPECT_EQ(GUID, IndexedMemProfRecord::getGUID("_Z3fooi"));
762+
763+
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
764+
MemProfData.Frames);
765+
CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
766+
MemProfData.CallStacks, FrameIdConv);
767+
MemProfRecord Record = IndexedRecord.toMemProfRecord(CSIdConv);
768+
787769
ASSERT_THAT(Record.AllocSites, SizeIs(1));
788-
EXPECT_EQ(Record.AllocSites[0].CSId, hashCallStack(CS1));
789-
EXPECT_THAT(Record.CallSiteIds, IsEmpty());
770+
EXPECT_THAT(Record.AllocSites[0].CallStack,
771+
ElementsAre(Frame(0x100, 11, 10, true)));
772+
EXPECT_THAT(Record.CallSites, IsEmpty());
790773
}
791774

792775
template <typename T> std::string serializeInYAML(T &Val) {

0 commit comments

Comments
 (0)