Skip to content

Commit e14827f

Browse files
teresajohnsonteresajohnson
authored
[MemProf] Templatize CallStackRadixTreeBuilder (NFC) (#117014)
Prepare for usage in the bitcode reader/writer where we already have a LinearFrameId: - templatize input frame id type in CallStackRadixTreeBuilder - templatize input frame id type in computeFrameHistogram - make the map from FrameId to LinearFrameId optional We plan to use the same radix format in the ThinLTO summary records, where we already have a LinearFrameId.
1 parent 6473a36 commit e14827f

File tree

4 files changed

+52
-35
lines changed

4 files changed

+52
-35
lines changed

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,8 +1050,9 @@ struct FrameStat {
10501050
};
10511051

10521052
// Compute a histogram of Frames in call stacks.
1053-
llvm::DenseMap<FrameId, FrameStat>
1054-
computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
1053+
template <typename FrameIdTy>
1054+
llvm::DenseMap<FrameIdTy, FrameStat>
1055+
computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
10551056
&MemProfCallStackData);
10561057

10571058
// Construct a radix tree of call stacks.
@@ -1109,7 +1110,7 @@ computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
11091110
// On-disk IndexedMemProfRecord will refer to call stacks by their indexes into
11101111
// the radix tree array, so we do not explicitly encode mappings like:
11111112
// "CallStackId 1 -> 11".
1112-
class CallStackRadixTreeBuilder {
1113+
template <typename FrameIdTy> class CallStackRadixTreeBuilder {
11131114
// The radix tree array.
11141115
std::vector<LinearFrameId> RadixArray;
11151116

@@ -1136,23 +1137,25 @@ class CallStackRadixTreeBuilder {
11361137
// RadixArray[Indexes[5 - 1]] is the last frame of the common prefix.
11371138
std::vector<LinearCallStackId> Indexes;
11381139

1139-
using CSIdPair = std::pair<CallStackId, llvm::SmallVector<FrameId>>;
1140+
using CSIdPair = std::pair<CallStackId, llvm::SmallVector<FrameIdTy>>;
11401141

11411142
// Encode a call stack into RadixArray. Return the starting index within
11421143
// RadixArray.
1143-
LinearCallStackId encodeCallStack(
1144-
const llvm::SmallVector<FrameId> *CallStack,
1145-
const llvm::SmallVector<FrameId> *Prev,
1146-
const llvm::DenseMap<FrameId, LinearFrameId> &MemProfFrameIndexes);
1144+
LinearCallStackId
1145+
encodeCallStack(const llvm::SmallVector<FrameIdTy> *CallStack,
1146+
const llvm::SmallVector<FrameIdTy> *Prev,
1147+
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
1148+
MemProfFrameIndexes);
11471149

11481150
public:
11491151
CallStackRadixTreeBuilder() = default;
11501152

11511153
// Build a radix tree array.
1152-
void build(llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
1154+
void build(llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
11531155
&&MemProfCallStackData,
1154-
const llvm::DenseMap<FrameId, LinearFrameId> &MemProfFrameIndexes,
1155-
llvm::DenseMap<FrameId, FrameStat> &FrameHistogram);
1156+
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
1157+
MemProfFrameIndexes,
1158+
llvm::DenseMap<FrameIdTy, FrameStat> &FrameHistogram);
11561159

11571160
ArrayRef<LinearFrameId> getRadixArray() const { return RadixArray; }
11581161

‎llvm/lib/ProfileData/InstrProfWriter.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ writeMemProfCallStackArray(
635635
llvm::DenseMap<memprof::CallStackId, memprof::LinearCallStackId>
636636
MemProfCallStackIndexes;
637637

638-
memprof::CallStackRadixTreeBuilder Builder;
638+
memprof::CallStackRadixTreeBuilder<memprof::FrameId> Builder;
639639
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
640640
FrameHistogram);
641641
for (auto I : Builder.getRadixArray())

‎llvm/lib/ProfileData/MemProf.cpp‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,12 @@ CallStackId hashCallStack(ArrayRef<FrameId> CS) {
436436
// To quickly determine the location of the common prefix within RadixArray,
437437
// Indexes caches the indexes of the previous call stack's frames within
438438
// RadixArray.
439-
LinearCallStackId CallStackRadixTreeBuilder::encodeCallStack(
440-
const llvm::SmallVector<FrameId> *CallStack,
441-
const llvm::SmallVector<FrameId> *Prev,
442-
const llvm::DenseMap<FrameId, LinearFrameId> &MemProfFrameIndexes) {
439+
template <typename FrameIdTy>
440+
LinearCallStackId CallStackRadixTreeBuilder<FrameIdTy>::encodeCallStack(
441+
const llvm::SmallVector<FrameIdTy> *CallStack,
442+
const llvm::SmallVector<FrameIdTy> *Prev,
443+
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
444+
MemProfFrameIndexes) {
443445
// Compute the length of the common root prefix between Prev and CallStack.
444446
uint32_t CommonLen = 0;
445447
if (Prev) {
@@ -464,10 +466,11 @@ LinearCallStackId CallStackRadixTreeBuilder::encodeCallStack(
464466

465467
// Copy the part of the call stack beyond the common prefix to RadixArray.
466468
assert(CommonLen <= CallStack->size());
467-
for (FrameId F : llvm::drop_begin(llvm::reverse(*CallStack), CommonLen)) {
469+
for (FrameIdTy F : llvm::drop_begin(llvm::reverse(*CallStack), CommonLen)) {
468470
// Remember the index of F in RadixArray.
469471
Indexes.push_back(RadixArray.size());
470-
RadixArray.push_back(MemProfFrameIndexes.find(F)->second);
472+
RadixArray.push_back(
473+
MemProfFrameIndexes ? MemProfFrameIndexes->find(F)->second : F);
471474
}
472475
assert(CallStack->size() == Indexes.size());
473476

@@ -479,11 +482,13 @@ LinearCallStackId CallStackRadixTreeBuilder::encodeCallStack(
479482
return RadixArray.size() - 1;
480483
}
481484

482-
void CallStackRadixTreeBuilder::build(
483-
llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
485+
template <typename FrameIdTy>
486+
void CallStackRadixTreeBuilder<FrameIdTy>::build(
487+
llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
484488
&&MemProfCallStackData,
485-
const llvm::DenseMap<FrameId, LinearFrameId> &MemProfFrameIndexes,
486-
llvm::DenseMap<FrameId, FrameStat> &FrameHistogram) {
489+
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
490+
MemProfFrameIndexes,
491+
llvm::DenseMap<FrameIdTy, FrameStat> &FrameHistogram) {
487492
// Take the vector portion of MemProfCallStackData. The vector is exactly
488493
// what we need to sort. Also, we no longer need its lookup capability.
489494
llvm::SmallVector<CSIdPair, 0> CallStacks = MemProfCallStackData.takeVector();
@@ -535,7 +540,7 @@ void CallStackRadixTreeBuilder::build(
535540
// root.
536541
return std::lexicographical_compare(
537542
L.second.rbegin(), L.second.rend(), R.second.rbegin(), R.second.rend(),
538-
[&](FrameId F1, FrameId F2) {
543+
[&](FrameIdTy F1, FrameIdTy F2) {
539544
uint64_t H1 = FrameHistogram[F1].Count;
540545
uint64_t H2 = FrameHistogram[F2].Count;
541546
// Popular frames should come later because we encode call stacks from
@@ -585,7 +590,7 @@ void CallStackRadixTreeBuilder::build(
585590
// traverse CallStacks in the reverse order, then Call Stack 3 has the
586591
// complete call stack encoded without any pointers. Call Stack 1 and 2 point
587592
// to appropriate prefixes of Call Stack 3.
588-
const llvm::SmallVector<FrameId> *Prev = nullptr;
593+
const llvm::SmallVector<FrameIdTy> *Prev = nullptr;
589594
for (const auto &[CSId, CallStack] : llvm::reverse(CallStacks)) {
590595
LinearCallStackId Pos =
591596
encodeCallStack(&CallStack, Prev, MemProfFrameIndexes);
@@ -608,10 +613,14 @@ void CallStackRadixTreeBuilder::build(
608613
V = RadixArray.size() - 1 - V;
609614
}
610615

611-
llvm::DenseMap<FrameId, FrameStat>
612-
computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
616+
// Explicitly instantiate class with the utilized FrameIdTy.
617+
template class CallStackRadixTreeBuilder<FrameId>;
618+
619+
template <typename FrameIdTy>
620+
llvm::DenseMap<FrameIdTy, FrameStat>
621+
computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
613622
&MemProfCallStackData) {
614-
llvm::DenseMap<FrameId, FrameStat> Histogram;
623+
llvm::DenseMap<FrameIdTy, FrameStat> Histogram;
615624

616625
for (const auto &KV : MemProfCallStackData) {
617626
const auto &CS = KV.second;
@@ -624,6 +633,11 @@ computeFrameHistogram(llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
624633
return Histogram;
625634
}
626635

636+
// Explicitly instantiate function with the utilized FrameIdTy.
637+
template llvm::DenseMap<FrameId, FrameStat> computeFrameHistogram<FrameId>(
638+
llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>>
639+
&MemProfCallStackData);
640+
627641
void verifyIndexedMemProfRecord(const IndexedMemProfRecord &Record) {
628642
for (const auto &AS : Record.AllocSites) {
629643
assert(AS.CSId == hashCallStack(AS.CallStack));

‎llvm/unittests/ProfileData/MemProfTest.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -659,8 +659,8 @@ TEST(MemProf, RadixTreeBuilderEmpty) {
659659
llvm::MapVector<CallStackId, llvm::SmallVector<FrameId>> MemProfCallStackData;
660660
llvm::DenseMap<llvm::memprof::FrameId, llvm::memprof::FrameStat>
661661
FrameHistogram =
662-
llvm::memprof::computeFrameHistogram(MemProfCallStackData);
663-
llvm::memprof::CallStackRadixTreeBuilder Builder;
662+
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
663+
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
664664
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
665665
FrameHistogram);
666666
ASSERT_THAT(Builder.getRadixArray(), testing::IsEmpty());
@@ -677,8 +677,8 @@ TEST(MemProf, RadixTreeBuilderOne) {
677677
MemProfCallStackData.insert({llvm::memprof::hashCallStack(CS1), CS1});
678678
llvm::DenseMap<llvm::memprof::FrameId, llvm::memprof::FrameStat>
679679
FrameHistogram =
680-
llvm::memprof::computeFrameHistogram(MemProfCallStackData);
681-
llvm::memprof::CallStackRadixTreeBuilder Builder;
680+
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
681+
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
682682
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
683683
FrameHistogram);
684684
EXPECT_THAT(Builder.getRadixArray(), testing::ElementsAreArray({
@@ -704,8 +704,8 @@ TEST(MemProf, RadixTreeBuilderTwo) {
704704
MemProfCallStackData.insert({llvm::memprof::hashCallStack(CS2), CS2});
705705
llvm::DenseMap<llvm::memprof::FrameId, llvm::memprof::FrameStat>
706706
FrameHistogram =
707-
llvm::memprof::computeFrameHistogram(MemProfCallStackData);
708-
llvm::memprof::CallStackRadixTreeBuilder Builder;
707+
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
708+
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
709709
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
710710
FrameHistogram);
711711
EXPECT_THAT(Builder.getRadixArray(),
@@ -742,8 +742,8 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
742742
MemProfCallStackData.insert({llvm::memprof::hashCallStack(CS4), CS4});
743743
llvm::DenseMap<llvm::memprof::FrameId, llvm::memprof::FrameStat>
744744
FrameHistogram =
745-
llvm::memprof::computeFrameHistogram(MemProfCallStackData);
746-
llvm::memprof::CallStackRadixTreeBuilder Builder;
745+
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
746+
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
747747
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
748748
FrameHistogram);
749749
EXPECT_THAT(Builder.getRadixArray(),

0 commit comments

Comments
 (0)