Skip to content

Commit b2d27a4

Browse files
kazutakahiratakazutakahirata
committed
[memprof] Use return values from addFrame and addCallStack (NFC)
Migrating away from Frame::hash and hashCallStack further encapsulates how the IDs are calculated. Note that unit tests are the only places where Frame::hash and hashCallStack are used. The code proper (i.e. llvm/lib) uses IndexedMemProfData::{addFrame,addCallStack}; they do not directly use Frame::hash or hashCallStack.
1 parent b3cba9b commit b2d27a4

File tree

1 file changed

+37
-41
lines changed

1 file changed

+37
-41
lines changed

‎llvm/unittests/ProfileData/MemProfTest.cpp‎

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -411,10 +411,10 @@ TEST(MemProf, BaseMemProfReader) {
411411
/*Column=*/5, /*IsInlineFrame=*/true);
412412
Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
413413
/*Column=*/2, /*IsInlineFrame=*/false);
414-
MemProfData.addFrame(F1);
415-
MemProfData.addFrame(F2);
414+
auto F1Id = MemProfData.addFrame(F1);
415+
auto F2Id = MemProfData.addFrame(F2);
416416

417-
llvm::SmallVector<FrameId> CallStack{F1.hash(), F2.hash()};
417+
llvm::SmallVector<FrameId> CallStack{F1Id, F2Id};
418418
CallStackId CSId = MemProfData.addCallStack(std::move(CallStack));
419419

420420
IndexedMemProfRecord FakeRecord;
@@ -443,19 +443,17 @@ TEST(MemProf, BaseMemProfReaderWithCSIdMap) {
443443
/*Column=*/5, /*IsInlineFrame=*/true);
444444
Frame F2(/*Hash=*/IndexedMemProfRecord::getGUID("bar"), /*LineOffset=*/10,
445445
/*Column=*/2, /*IsInlineFrame=*/false);
446-
MemProfData.addFrame(F1);
447-
MemProfData.addFrame(F2);
446+
auto F1Id = MemProfData.addFrame(F1);
447+
auto F2Id = MemProfData.addFrame(F2);
448448

449-
llvm::SmallVector<FrameId> CallStack = {F1.hash(), F2.hash()};
450-
MemProfData.addCallStack(CallStack);
449+
llvm::SmallVector<FrameId> CallStack = {F1Id, F2Id};
450+
auto CSId = MemProfData.addCallStack(CallStack);
451451

452452
IndexedMemProfRecord FakeRecord;
453453
MemInfoBlock Block;
454454
Block.AllocCount = 1U, Block.TotalAccessDensity = 4,
455455
Block.TotalLifetime = 200001;
456-
FakeRecord.AllocSites.emplace_back(
457-
/*CSId=*/hashCallStack(CallStack),
458-
/*MB=*/Block);
456+
FakeRecord.AllocSites.emplace_back(/*CSId=*/CSId, /*MB=*/Block);
459457
MemProfData.Records.insert({F1.hash(), FakeRecord});
460458

461459
MemProfReader Reader(std::move(MemProfData));
@@ -480,28 +478,28 @@ TEST(MemProf, IndexedMemProfRecordToMemProfRecord) {
480478
Frame F2(2, 0, 0, false);
481479
Frame F3(3, 0, 0, false);
482480
Frame F4(4, 0, 0, false);
483-
MemProfData.addFrame(F1);
484-
MemProfData.addFrame(F2);
485-
MemProfData.addFrame(F3);
486-
MemProfData.addFrame(F4);
487-
488-
llvm::SmallVector<FrameId> CS1 = {F1.hash(), F2.hash()};
489-
llvm::SmallVector<FrameId> CS2 = {F1.hash(), F3.hash()};
490-
llvm::SmallVector<FrameId> CS3 = {F2.hash(), F3.hash()};
491-
llvm::SmallVector<FrameId> CS4 = {F2.hash(), F4.hash()};
492-
MemProfData.addCallStack(CS1);
493-
MemProfData.addCallStack(CS2);
494-
MemProfData.addCallStack(CS3);
495-
MemProfData.addCallStack(CS4);
481+
auto F1Id = MemProfData.addFrame(F1);
482+
auto F2Id = MemProfData.addFrame(F2);
483+
auto F3Id = MemProfData.addFrame(F3);
484+
auto F4Id = MemProfData.addFrame(F4);
485+
486+
llvm::SmallVector<FrameId> CS1 = {F1Id, F2Id};
487+
llvm::SmallVector<FrameId> CS2 = {F1Id, F3Id};
488+
llvm::SmallVector<FrameId> CS3 = {F2Id, F3Id};
489+
llvm::SmallVector<FrameId> CS4 = {F2Id, F4Id};
490+
auto CS1Id = MemProfData.addCallStack(CS1);
491+
auto CS2Id = MemProfData.addCallStack(CS2);
492+
auto CS3Id = MemProfData.addCallStack(CS3);
493+
auto CS4Id = MemProfData.addCallStack(CS4);
496494

497495
IndexedMemProfRecord IndexedRecord;
498496
IndexedAllocationInfo AI;
499-
AI.CSId = hashCallStack(CS1);
497+
AI.CSId = CS1Id;
500498
IndexedRecord.AllocSites.push_back(AI);
501-
AI.CSId = hashCallStack(CS2);
499+
AI.CSId = CS2Id;
502500
IndexedRecord.AllocSites.push_back(AI);
503-
IndexedRecord.CallSiteIds.push_back(hashCallStack(CS3));
504-
IndexedRecord.CallSiteIds.push_back(hashCallStack(CS4));
501+
IndexedRecord.CallSiteIds.push_back(CS3Id);
502+
IndexedRecord.CallSiteIds.push_back(CS4Id);
505503

506504
FrameIdConverter<decltype(MemProfData.Frames)> FrameIdConv(
507505
MemProfData.Frames);
@@ -598,7 +596,7 @@ TEST(MemProf, RadixTreeBuilderOne) {
598596
{11, 1}, {12, 2}, {13, 3}};
599597
llvm::SmallVector<FrameId> CS1 = {13, 12, 11};
600598
IndexedMemProfData MemProfData;
601-
MemProfData.addCallStack(CS1);
599+
auto CS1Id = MemProfData.addCallStack(CS1);
602600
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
603601
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
604602
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -611,7 +609,7 @@ TEST(MemProf, RadixTreeBuilderOne) {
611609
1U // MemProfFrameIndexes[11]
612610
));
613611
const auto Mappings = Builder.takeCallStackPos();
614-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U)));
612+
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(CS1Id, 0U)));
615613
}
616614

617615
// Verify CallStackRadixTreeBuilder can form a link between two call stacks.
@@ -621,8 +619,8 @@ TEST(MemProf, RadixTreeBuilderTwo) {
621619
llvm::SmallVector<FrameId> CS1 = {12, 11};
622620
llvm::SmallVector<FrameId> CS2 = {13, 12, 11};
623621
IndexedMemProfData MemProfData;
624-
MemProfData.addCallStack(CS1);
625-
MemProfData.addCallStack(CS2);
622+
auto CS1Id = MemProfData.addCallStack(CS1);
623+
auto CS2Id = MemProfData.addCallStack(CS2);
626624
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
627625
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
628626
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -637,8 +635,7 @@ TEST(MemProf, RadixTreeBuilderTwo) {
637635
1U // MemProfFrameIndexes[11]
638636
));
639637
const auto Mappings = Builder.takeCallStackPos();
640-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U),
641-
Pair(hashCallStack(CS2), 2U)));
638+
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(CS1Id, 0U), Pair(CS2Id, 2U)));
642639
}
643640

644641
// Verify CallStackRadixTreeBuilder can form a jump to a prefix that itself has
@@ -652,10 +649,10 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
652649
llvm::SmallVector<FrameId> CS3 = {17, 16, 12, 11};
653650
llvm::SmallVector<FrameId> CS4 = {18, 16, 12, 11};
654651
IndexedMemProfData MemProfData;
655-
MemProfData.addCallStack(CS1);
656-
MemProfData.addCallStack(CS2);
657-
MemProfData.addCallStack(CS3);
658-
MemProfData.addCallStack(CS4);
652+
auto CS1Id = MemProfData.addCallStack(CS1);
653+
auto CS2Id = MemProfData.addCallStack(CS2);
654+
auto CS3Id = MemProfData.addCallStack(CS3);
655+
auto CS4Id = MemProfData.addCallStack(CS4);
659656
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
660657
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
661658
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -679,10 +676,9 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
679676
1U // MemProfFrameIndexes[11]
680677
));
681678
const auto Mappings = Builder.takeCallStackPos();
682-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U),
683-
Pair(hashCallStack(CS2), 3U),
684-
Pair(hashCallStack(CS3), 7U),
685-
Pair(hashCallStack(CS4), 10U)));
679+
EXPECT_THAT(Mappings,
680+
UnorderedElementsAre(Pair(CS1Id, 0U), Pair(CS2Id, 3U),
681+
Pair(CS3Id, 7U), Pair(CS4Id, 10U)));
686682
}
687683

688684
// Verify that we can parse YAML and retrieve IndexedMemProfData as expected.

0 commit comments

Comments
 (0)