Skip to content

[MemProf] Use radix tree for alloc contexts in bitcode summaries #117066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,12 @@ enum GlobalValueSummarySymtabCodes {
// [valueid, n x stackidindex]
FS_PERMODULE_CALLSITE_INFO = 26,
// Summary of per-module allocation memprof metadata.
// [nummib, nummib x (alloc type, numstackids, numstackids x stackidindex),
// [nummib, nummib x (alloc type, context radix tree index),
// [nummib x (numcontext x total size)]?]
FS_PERMODULE_ALLOC_INFO = 27,
// Summary of combined index memprof callsite metadata.
// [valueid, numstackindices, numver,
// numstackindices x stackidindex, numver x version]
// [valueid, context radix tree index, numver,
// numver x version]
FS_COMBINED_CALLSITE_INFO = 28,
// Summary of combined index allocation memprof metadata.
// [nummib, numver,
Expand All @@ -331,6 +331,10 @@ enum GlobalValueSummarySymtabCodes {
// the entries must be in the exact same order as the corresponding sizes.
// [nummib x (numcontext x full stack id)]
FS_ALLOC_CONTEXT_IDS = 31,
// Linearized radix tree of allocation contexts. See the description above the
// CallStackRadixTreeBuilder class in ProfileData/MemProf.h for format.
// [n x entry]
FS_CONTEXT_RADIX_TREE_ARRAY = 32,
};

enum MetadataCodes {
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ GetCodeName(unsigned CodeID, unsigned BlockID,
STRINGIFY_CODE(FS, COMBINED_ALLOC_INFO)
STRINGIFY_CODE(FS, STACK_IDS)
STRINGIFY_CODE(FS, ALLOC_CONTEXT_IDS)
STRINGIFY_CODE(FS, CONTEXT_RADIX_TREE_ARRAY)
}
case bitc::METADATA_ATTACHMENT_ID:
switch (CodeID) {
Expand Down
71 changes: 55 additions & 16 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,10 @@ class ModuleSummaryIndexBitcodeReader : public BitcodeReaderBase {
/// ids from the lists in the callsite and alloc entries to the index.
std::vector<uint64_t> StackIds;

/// Linearized radix tree of allocation contexts. See the description above
/// the CallStackRadixTreeBuilder class in ProfileData/MemProf.h for format.
std::vector<uint64_t> RadixArray;

public:
ModuleSummaryIndexBitcodeReader(
BitstreamCursor Stream, StringRef Strtab, ModuleSummaryIndex &TheIndex,
Expand All @@ -1013,6 +1017,8 @@ class ModuleSummaryIndexBitcodeReader : public BitcodeReaderBase {
TypeIdCompatibleVtableInfo &TypeId);
std::vector<FunctionSummary::ParamAccess>
parseParamAccesses(ArrayRef<uint64_t> Record);
SmallVector<unsigned> parseAllocInfoContext(ArrayRef<uint64_t> Record,
unsigned &I);

template <bool AllowNullValueInfo = false>
std::pair<ValueInfo, GlobalValue::GUID>
Expand Down Expand Up @@ -7544,6 +7550,48 @@ void ModuleSummaryIndexBitcodeReader::parseTypeIdCompatibleVtableSummaryRecord(
parseTypeIdCompatibleVtableInfo(Record, Slot, TypeId);
}

SmallVector<unsigned> ModuleSummaryIndexBitcodeReader::parseAllocInfoContext(
ArrayRef<uint64_t> Record, unsigned &I) {
SmallVector<unsigned> StackIdList;
// For backwards compatibility with old format before radix tree was
// used, simply see if we found a radix tree array record (and thus if
// the RadixArray is non-empty).
if (RadixArray.empty()) {
unsigned NumStackEntries = Record[I++];
assert(Record.size() - I >= NumStackEntries);
StackIdList.reserve(NumStackEntries);
for (unsigned J = 0; J < NumStackEntries; J++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you are copying this block of code from elsewhere, but may I suggest reserve here?

Suggested change
for (unsigned J = 0; J < NumStackEntries; J++) {
StackIdList.reserve(NumStackEntries);
for (unsigned J = 0; J < NumStackEntries; J++) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

assert(Record[I] < StackIds.size());
StackIdList.push_back(
TheIndex.addOrGetStackIdIndex(StackIds[Record[I++]]));
}
} else {
unsigned RadixIndex = Record[I++];
// See the comments above CallStackRadixTreeBuilder in ProfileData/MemProf.h
// for a detailed description of the radix tree array format. Briefly, the
// first entry will be the number of frames, any negative values are the
// negative of the offset of the next frame, and otherwise the frames are in
// increasing linear order.
assert(RadixIndex < RadixArray.size());
unsigned NumStackIds = RadixArray[RadixIndex++];
StackIdList.reserve(NumStackIds);
while (NumStackIds--) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, may I suggest reserve here? The main reason for encoding the length in the radix tree array was to enable reserve.

Suggested change
while (NumStackIds--) {
StackIdList.reserve(NumStackIds);
while (NumStackIds--) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

assert(RadixIndex < RadixArray.size());
unsigned Elem = RadixArray[RadixIndex];
if (static_cast<std::make_signed_t<unsigned>>(Elem) < 0) {
RadixIndex = RadixIndex - Elem;
assert(RadixIndex < RadixArray.size());
Elem = RadixArray[RadixIndex];
// We shouldn't encounter a second offset in a row.
assert(static_cast<std::make_signed_t<unsigned>>(Elem) >= 0);
}
RadixIndex++;
StackIdList.push_back(TheIndex.addOrGetStackIdIndex(StackIds[Elem]));
}
}
return StackIdList;
}

static void setSpecialRefs(SmallVectorImpl<ValueInfo> &Refs, unsigned ROCnt,
unsigned WOCnt) {
// Readonly and writeonly refs are in the end of the refs list.
Expand Down Expand Up @@ -8010,6 +8058,11 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
break;
}

case bitc::FS_CONTEXT_RADIX_TREE_ARRAY: { // [n x entry]
RadixArray = ArrayRef<uint64_t>(Record);
break;
}

case bitc::FS_PERMODULE_CALLSITE_INFO: {
unsigned ValueID = Record[0];
SmallVector<unsigned> StackIdList;
Expand Down Expand Up @@ -8065,14 +8118,7 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
(Version < 10 && I < Record.size())) {
assert(Record.size() - I >= 2);
AllocationType AllocType = (AllocationType)Record[I++];
unsigned NumStackEntries = Record[I++];
assert(Record.size() - I >= NumStackEntries);
SmallVector<unsigned> StackIdList;
for (unsigned J = 0; J < NumStackEntries; J++) {
assert(Record[I] < StackIds.size());
StackIdList.push_back(
TheIndex.addOrGetStackIdIndex(StackIds[Record[I++]]));
}
auto StackIdList = parseAllocInfoContext(Record, I);
MIBs.push_back(MIBInfo(AllocType, std::move(StackIdList)));
}
// We either have nothing left or at least NumMIBs context size info
Expand Down Expand Up @@ -8123,14 +8169,7 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
while (MIBsRead++ < NumMIBs) {
assert(Record.size() - I >= 2);
AllocationType AllocType = (AllocationType)Record[I++];
unsigned NumStackEntries = Record[I++];
assert(Record.size() - I >= NumStackEntries);
SmallVector<unsigned> StackIdList;
for (unsigned J = 0; J < NumStackEntries; J++) {
assert(Record[I] < StackIds.size());
StackIdList.push_back(
TheIndex.addOrGetStackIdIndex(StackIds[Record[I++]]));
}
auto StackIdList = parseAllocInfoContext(Record, I);
MIBs.push_back(MIBInfo(AllocType, std::move(StackIdList)));
}
assert(Record.size() - I >= NumVersions);
Expand Down
Loading
Loading