Skip to content

Commit 9b14ded

Browse files
kazutakahiratakazutakahirata
authored
[memprof] Use return values from addFrame and addCallStack (NFC) (llvm#119676)
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 f7e868f commit 9b14ded

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(std::move(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(std::move(CS1));
491+
auto CS2Id = MemProfData.addCallStack(std::move(CS2));
492+
auto CS3Id = MemProfData.addCallStack(std::move(CS3));
493+
auto CS4Id = MemProfData.addCallStack(std::move(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);
@@ -596,7 +594,7 @@ TEST(MemProf, RadixTreeBuilderOne) {
596594
{11, 1}, {12, 2}, {13, 3}};
597595
llvm::SmallVector<FrameId> CS1 = {13, 12, 11};
598596
IndexedMemProfData MemProfData;
599-
MemProfData.addCallStack(CS1);
597+
auto CS1Id = MemProfData.addCallStack(std::move(CS1));
600598
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
601599
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
602600
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -609,7 +607,7 @@ TEST(MemProf, RadixTreeBuilderOne) {
609607
1U // MemProfFrameIndexes[11]
610608
));
611609
const auto Mappings = Builder.takeCallStackPos();
612-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U)));
610+
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(CS1Id, 0U)));
613611
}
614612

615613
// Verify CallStackRadixTreeBuilder can form a link between two call stacks.
@@ -619,8 +617,8 @@ TEST(MemProf, RadixTreeBuilderTwo) {
619617
llvm::SmallVector<FrameId> CS1 = {12, 11};
620618
llvm::SmallVector<FrameId> CS2 = {13, 12, 11};
621619
IndexedMemProfData MemProfData;
622-
MemProfData.addCallStack(CS1);
623-
MemProfData.addCallStack(CS2);
620+
auto CS1Id = MemProfData.addCallStack(std::move(CS1));
621+
auto CS2Id = MemProfData.addCallStack(std::move(CS2));
624622
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
625623
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
626624
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -635,8 +633,7 @@ TEST(MemProf, RadixTreeBuilderTwo) {
635633
1U // MemProfFrameIndexes[11]
636634
));
637635
const auto Mappings = Builder.takeCallStackPos();
638-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U),
639-
Pair(hashCallStack(CS2), 2U)));
636+
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(CS1Id, 0U), Pair(CS2Id, 2U)));
640637
}
641638

642639
// Verify CallStackRadixTreeBuilder can form a jump to a prefix that itself has
@@ -650,10 +647,10 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
650647
llvm::SmallVector<FrameId> CS3 = {17, 16, 12, 11};
651648
llvm::SmallVector<FrameId> CS4 = {18, 16, 12, 11};
652649
IndexedMemProfData MemProfData;
653-
MemProfData.addCallStack(CS1);
654-
MemProfData.addCallStack(CS2);
655-
MemProfData.addCallStack(CS3);
656-
MemProfData.addCallStack(CS4);
650+
auto CS1Id = MemProfData.addCallStack(std::move(CS1));
651+
auto CS2Id = MemProfData.addCallStack(std::move(CS2));
652+
auto CS3Id = MemProfData.addCallStack(std::move(CS3));
653+
auto CS4Id = MemProfData.addCallStack(std::move(CS4));
657654
llvm::DenseMap<FrameId, FrameStat> FrameHistogram =
658655
computeFrameHistogram<FrameId>(MemProfData.CallStacks);
659656
CallStackRadixTreeBuilder<FrameId> Builder;
@@ -677,10 +674,9 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
677674
1U // MemProfFrameIndexes[11]
678675
));
679676
const auto Mappings = Builder.takeCallStackPos();
680-
EXPECT_THAT(Mappings, UnorderedElementsAre(Pair(hashCallStack(CS1), 0U),
681-
Pair(hashCallStack(CS2), 3U),
682-
Pair(hashCallStack(CS3), 7U),
683-
Pair(hashCallStack(CS4), 10U)));
677+
EXPECT_THAT(Mappings,
678+
UnorderedElementsAre(Pair(CS1Id, 0U), Pair(CS2Id, 3U),
679+
Pair(CS3Id, 7U), Pair(CS4Id, 10U)));
684680
}
685681

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

0 commit comments

Comments
 (0)