Skip to content

Commit e3426da

Browse files
[memprof] Improve CallStackRadixTreeBuilder::build
CallStackRadixTreeBuilder::build takes the parameter MemProfFrameIndexes by value, involving copies: std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>> MemProfFrameIndexes This patch changes the type to a pointer so that we don't have to make a copy every time we pass the argument.
1 parent 28064bf commit e3426da

File tree

5 files changed

+17
-20
lines changed

5 files changed

+17
-20
lines changed

llvm/include/llvm/ProfileData/MemProf.h

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,21 +1125,20 @@ template <typename FrameIdTy> class CallStackRadixTreeBuilder {
11251125

11261126
// Encode a call stack into RadixArray. Return the starting index within
11271127
// RadixArray.
1128-
LinearCallStackId
1129-
encodeCallStack(const llvm::SmallVector<FrameIdTy> *CallStack,
1130-
const llvm::SmallVector<FrameIdTy> *Prev,
1131-
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
1132-
MemProfFrameIndexes);
1128+
LinearCallStackId encodeCallStack(
1129+
const llvm::SmallVector<FrameIdTy> *CallStack,
1130+
const llvm::SmallVector<FrameIdTy> *Prev,
1131+
const llvm::DenseMap<FrameIdTy, LinearFrameId> *MemProfFrameIndexes);
11331132

11341133
public:
11351134
CallStackRadixTreeBuilder() = default;
11361135

11371136
// Build a radix tree array.
1138-
void build(llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
1139-
&&MemProfCallStackData,
1140-
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
1141-
MemProfFrameIndexes,
1142-
llvm::DenseMap<FrameIdTy, FrameStat> &FrameHistogram);
1137+
void
1138+
build(llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
1139+
&&MemProfCallStackData,
1140+
const llvm::DenseMap<FrameIdTy, LinearFrameId> *MemProfFrameIndexes,
1141+
llvm::DenseMap<FrameIdTy, FrameStat> &FrameHistogram);
11431142

11441143
ArrayRef<LinearFrameId> getRadixArray() const { return RadixArray; }
11451144

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4235,7 +4235,7 @@ static DenseMap<CallStackId, LinearCallStackId> writeMemoryProfileRadixTree(
42354235
CallStackRadixTreeBuilder<LinearFrameId> Builder;
42364236
// We don't need a MemProfFrameIndexes map as we have already converted the
42374237
// full stack id hash to a linear offset into the StackIds array.
4238-
Builder.build(std::move(CallStacks), /*MemProfFrameIndexes=*/std::nullopt,
4238+
Builder.build(std::move(CallStacks), /*MemProfFrameIndexes=*/nullptr,
42394239
FrameHistogram);
42404240
Stream.EmitRecord(bitc::FS_CONTEXT_RADIX_TREE_ARRAY, Builder.getRadixArray(),
42414241
RadixAbbrev);

llvm/lib/ProfileData/InstrProfWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ writeMemProfCallStackArray(
636636
MemProfCallStackIndexes;
637637

638638
memprof::CallStackRadixTreeBuilder<memprof::FrameId> Builder;
639-
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
639+
Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
640640
FrameHistogram);
641641
for (auto I : Builder.getRadixArray())
642642
OS.write32(I);

llvm/lib/ProfileData/MemProf.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -335,8 +335,7 @@ template <typename FrameIdTy>
335335
LinearCallStackId CallStackRadixTreeBuilder<FrameIdTy>::encodeCallStack(
336336
const llvm::SmallVector<FrameIdTy> *CallStack,
337337
const llvm::SmallVector<FrameIdTy> *Prev,
338-
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
339-
MemProfFrameIndexes) {
338+
const llvm::DenseMap<FrameIdTy, LinearFrameId> *MemProfFrameIndexes) {
340339
// Compute the length of the common root prefix between Prev and CallStack.
341340
uint32_t CommonLen = 0;
342341
if (Prev) {
@@ -381,8 +380,7 @@ template <typename FrameIdTy>
381380
void CallStackRadixTreeBuilder<FrameIdTy>::build(
382381
llvm::MapVector<CallStackId, llvm::SmallVector<FrameIdTy>>
383382
&&MemProfCallStackData,
384-
std::optional<const llvm::DenseMap<FrameIdTy, LinearFrameId>>
385-
MemProfFrameIndexes,
383+
const llvm::DenseMap<FrameIdTy, LinearFrameId> *MemProfFrameIndexes,
386384
llvm::DenseMap<FrameIdTy, FrameStat> &FrameHistogram) {
387385
// Take the vector portion of MemProfCallStackData. The vector is exactly
388386
// what we need to sort. Also, we no longer need its lookup capability.

llvm/unittests/ProfileData/MemProfTest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ TEST(MemProf, RadixTreeBuilderEmpty) {
628628
FrameHistogram =
629629
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
630630
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
631-
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
631+
Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
632632
FrameHistogram);
633633
ASSERT_THAT(Builder.getRadixArray(), testing::IsEmpty());
634634
const auto Mappings = Builder.takeCallStackPos();
@@ -646,7 +646,7 @@ TEST(MemProf, RadixTreeBuilderOne) {
646646
FrameHistogram =
647647
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
648648
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
649-
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
649+
Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
650650
FrameHistogram);
651651
EXPECT_THAT(Builder.getRadixArray(), testing::ElementsAreArray({
652652
3U, // Size of CS1,
@@ -673,7 +673,7 @@ TEST(MemProf, RadixTreeBuilderTwo) {
673673
FrameHistogram =
674674
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
675675
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
676-
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
676+
Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
677677
FrameHistogram);
678678
EXPECT_THAT(Builder.getRadixArray(),
679679
testing::ElementsAreArray({
@@ -711,7 +711,7 @@ TEST(MemProf, RadixTreeBuilderSuccessiveJumps) {
711711
FrameHistogram =
712712
llvm::memprof::computeFrameHistogram<FrameId>(MemProfCallStackData);
713713
llvm::memprof::CallStackRadixTreeBuilder<FrameId> Builder;
714-
Builder.build(std::move(MemProfCallStackData), MemProfFrameIndexes,
714+
Builder.build(std::move(MemProfCallStackData), &MemProfFrameIndexes,
715715
FrameHistogram);
716716
EXPECT_THAT(Builder.getRadixArray(),
717717
testing::ElementsAreArray({

0 commit comments

Comments
 (0)