[llvm] [ProfileData] Refactor SampleProfileNameTable into a polymorphic class hierarchy (NFC) (PR #210252)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 16 23:07:47 PDT 2026
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/210252
This patch refactors SampleProfileNameTable into an abstract base
class with concrete derived classes like LazySampleProfileNameTable
and EagerSampleProfileNameTable.
The motivation is twofold:
- I want each derived class to focus on one data representation
instead of using complex if-then-else. Plus, I'm planning to
introduce one more data representation [1].
- I want each class to be populated and ready for use as soon as it is
constructed. That is, there is no intermediate state like
"constructed but waiting to be populated".
Now, you might notice that the iterator uses virtual operator[]. I
would argue that this is acceptable. We have three places where we
iterate over the entire range of the name table entries. Two of
these, namely SampleProfileNameSet and NamesInProfile, construct their
own sets for membership queries -- "Is this symbol in the name
table?". I'm planning to bring those sets right into our class
hierarchy in a follow-up patch.
[1]: https://discourse.llvm.org/t/rfc-faster-sample-profile-loading/90957/8
Assisted-by: Antigravity
>From 859a636552ec9faae2364b36ec3a4bf5545fe24a Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Thu, 16 Jul 2026 22:17:33 -0700
Subject: [PATCH] [ProfileData] Refactor SampleProfileNameTable into a
polymorphic class hierarchy (NFC)
This patch refactors SampleProfileNameTable into an abstract base
class with concrete derived classes like LazySampleProfileNameTable
and EagerSampleProfileNameTable.
The motivation is twofold:
- I want each derived class to focus on one data representation
instead of using complex if-then-else. Plus, I'm planning to
introduce one more data representation [1].
- I want each class to be populated and ready for use as soon as it is
constructed. That is, there is no intermediate state like
"constructed but waiting to be populated".
Now, you might notice that the iterator uses virtual operator[]. I
would argue that this is acceptable. We have three places where we
iterate over the entire range of the name table entries. Two of
these, namely SampleProfileNameSet and NamesInProfile, construct their
own sets for membership queries -- "Is this symbol in the name
table?". I'm planning to bring those sets right into our class
hierarchy in a follow-up patch.
[1]: https://discourse.llvm.org/t/rfc-faster-sample-profile-loading/90957/8
Assisted-by: Antigravity
---
.../llvm/ProfileData/SampleProfReader.h | 121 ++++++++----------
llvm/lib/ProfileData/SampleProfReader.cpp | 20 ++-
2 files changed, 68 insertions(+), 73 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index 890c3db244758..0b75192a9d3aa 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -351,101 +351,85 @@ class SampleProfileReaderItaniumRemapper {
/// from the memory-mapped buffer. It enforces the exclusivity of these
/// two formats and provides a unified read-only container interface.
class SampleProfileNameTable {
- const uint8_t *Start = nullptr;
- size_t Size = 0;
- std::vector<FunctionId> Vec;
-
- /// Helper function to read a FunctionId (MD5 hash) from a raw buffer.
- static FunctionId readFunctionIdFromMD5(const uint8_t *Ptr) {
- using namespace support;
- return FunctionId(
- endian::read<uint64_t, unaligned>(Ptr, endianness::little));
- }
-
public:
- /// iterator is a lightweight, self-contained input iterator designed
- /// to stream FunctionId symbols from either the memory-mapped
- /// file buffer (lazy loading from the FixedMD5 layout) or from an eagerly
- /// loaded vector of FunctionId objects (fallback).
class iterator
: public llvm::iterator_facade_base<iterator, std::input_iterator_tag,
FunctionId, std::ptrdiff_t,
const FunctionId *, FunctionId> {
public:
- // Tag type to indicate the lazy name table layout.
- struct UseLazy_t {};
- static constexpr UseLazy_t UseLazy{};
iterator() = default;
+ iterator(const SampleProfileNameTable *Table, size_t Idx)
+ : Table(Table), Idx(Idx) {}
- // Constructor for lazy loading.
- iterator(const uint8_t *P, UseLazy_t) : Ptr(P), IsLazy(true) {}
-
- // Constructor for eagerly loaded name table.
- iterator(const FunctionId *P)
- : Ptr(reinterpret_cast<const uint8_t *>(P)), IsLazy(false) {}
-
- bool operator==(const iterator &RHS) const { return Ptr == RHS.Ptr; }
+ bool operator==(const iterator &RHS) const {
+ return Table == RHS.Table && Idx == RHS.Idx;
+ }
iterator &operator++() {
- Ptr += IsLazy ? sizeof(uint64_t) : sizeof(FunctionId);
+ ++Idx;
return *this;
}
FunctionId operator*() const {
- return IsLazy ? readFunctionIdFromMD5(Ptr)
- : *reinterpret_cast<const FunctionId *>(Ptr);
+ assert(Table && Idx < Table->size() &&
+ "Dereferencing invalid or out-of-bounds iterator");
+ return (*Table)[Idx];
}
private:
- const uint8_t *Ptr = nullptr;
- bool IsLazy = false;
+ const SampleProfileNameTable *Table = nullptr;
+ size_t Idx = 0;
};
using const_iterator = iterator;
SampleProfileNameTable() = default;
+ SampleProfileNameTable(const SampleProfileNameTable &) = delete;
+ SampleProfileNameTable(SampleProfileNameTable &&) = delete;
+ SampleProfileNameTable &operator=(const SampleProfileNameTable &) = delete;
+ SampleProfileNameTable &operator=(SampleProfileNameTable &&) = delete;
+ virtual ~SampleProfileNameTable() = default;
- void clear() {
- Start = nullptr;
- Size = 0;
- Vec.clear();
- }
+ virtual size_t size() const = 0;
+ bool empty() const { return size() == 0; }
+ virtual FunctionId operator[](size_t Idx) const = 0;
- /// Transitions the table to lazy-loading mode, pointing directly to a
- /// contiguous buffer of little-endian 64-bit MD5 hashes.
- void setLazy(const uint8_t *S, size_t Sz) {
- clear();
- Start = S;
- Size = Sz;
- }
+ iterator begin() const { return iterator(this, 0); }
+ iterator end() const { return iterator(this, size()); }
+};
- /// Transitions the table to eager-loading mode by clearing previous state and
- /// returning a mutable reference to the underlying vector for population.
- std::vector<FunctionId> &setToEager() {
- clear();
- return Vec;
- }
+class LazySampleProfileNameTable final : public SampleProfileNameTable {
+ const uint8_t *Start = nullptr;
+ size_t Size = 0;
- size_t size() const { return Start ? Size : Vec.size(); }
- bool empty() const { return size() == 0; }
+public:
+ LazySampleProfileNameTable(const uint8_t *Start, size_t Size)
+ : Start(Start), Size(Size) {}
- FunctionId operator[](size_t Idx) const {
- assert(Idx < size());
- if (Start)
- return readFunctionIdFromMD5(Start + Idx * sizeof(uint64_t));
- return Vec[Idx];
- }
+ size_t size() const override { return Size; }
- iterator begin() const {
- if (Start)
- return {Start, iterator::UseLazy};
- return {Vec.data()};
+ FunctionId operator[](size_t Idx) const override {
+ assert(Idx < Size && "Index out of bounds");
+ using namespace support;
+ return FunctionId(endian::read<uint64_t, unaligned>(
+ Start + Idx * sizeof(uint64_t), endianness::little));
}
+};
+
+class EagerSampleProfileNameTable final : public SampleProfileNameTable {
+ std::vector<FunctionId> Vec;
- iterator end() const {
- if (Start)
- return {Start + Size * sizeof(uint64_t), iterator::UseLazy};
- return {Vec.data() + Vec.size()};
+public:
+ explicit EagerSampleProfileNameTable(std::vector<FunctionId> &&Vec)
+ : Vec(std::move(Vec)) {}
+ explicit EagerSampleProfileNameTable(const std::vector<FunctionId> &Vec)
+ : Vec(Vec) {}
+
+ size_t size() const override { return Vec.size(); }
+
+ FunctionId operator[](size_t Idx) const override {
+ assert(Idx < Vec.size() && "Index out of bounds");
+ return Vec[Idx];
}
};
@@ -768,7 +752,10 @@ class LLVM_ABI SampleProfileReaderBinary : public SampleProfileReader {
/// or inline instance.
llvm::iterator_range<SampleProfileNameTable::iterator>
getNameTable() const override {
- return {NameTable.begin(), NameTable.end()};
+ if (!NameTable)
+ return {SampleProfileNameTable::iterator(),
+ SampleProfileNameTable::iterator()};
+ return {NameTable->begin(), NameTable->end()};
}
protected:
@@ -838,7 +825,7 @@ class LLVM_ABI SampleProfileReaderBinary : public SampleProfileReader {
const uint8_t *End = nullptr;
/// Function name table.
- SampleProfileNameTable NameTable;
+ std::unique_ptr<SampleProfileNameTable> NameTable;
/// CSNameTable is used to save full context vectors. It is the backing buffer
/// for SampleContextFrames.
diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index b28b3406710d0..ef17d5102ee5a 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -604,12 +604,14 @@ inline ErrorOr<size_t> SampleProfileReaderBinary::readStringIndex(T &Table) {
ErrorOr<FunctionId>
SampleProfileReaderBinary::readStringFromTable(size_t *RetIdx) {
- auto Idx = readStringIndex(NameTable);
+ if (!NameTable)
+ return sampleprof_error::truncated_name_table;
+ auto Idx = readStringIndex(*NameTable);
if (std::error_code EC = Idx.getError())
return EC;
if (RetIdx)
*RetIdx = *Idx;
- return NameTable[*Idx];
+ return (*NameTable)[*Idx];
}
ErrorOr<SampleContextFrames>
@@ -1242,7 +1244,7 @@ std::error_code SampleProfileReaderBinary::readNameTable() {
// because optimization passes can only handle either type.
bool UseMD5 = useMD5();
- auto &TableVec = NameTable.setToEager();
+ std::vector<FunctionId> TableVec;
TableVec.reserve(*Size);
if (!ProfileIsCS) {
MD5SampleContextTable.clear();
@@ -1268,6 +1270,8 @@ std::error_code SampleProfileReaderBinary::readNameTable() {
}
if (!ProfileIsCS)
MD5SampleContextStart = MD5SampleContextTable.data();
+ NameTable =
+ std::make_unique<EagerSampleProfileNameTable>(std::move(TableVec));
return sampleprof_error::success;
}
@@ -1288,9 +1292,9 @@ SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
return sampleprof_error::truncated;
if (LazyLoadNameTable) {
- NameTable.setLazy(Data, *Size);
+ NameTable = std::make_unique<LazySampleProfileNameTable>(Data, *Size);
} else {
- auto &TableVec = NameTable.setToEager();
+ std::vector<FunctionId> TableVec;
TableVec.reserve(*Size);
for (size_t I = 0; I < *Size; ++I) {
using namespace support;
@@ -1298,6 +1302,8 @@ SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
Data + I * sizeof(uint64_t), endianness::little);
TableVec.emplace_back(FunctionId(FID));
}
+ NameTable =
+ std::make_unique<EagerSampleProfileNameTable>(std::move(TableVec));
}
if (!ProfileIsCS)
MD5SampleContextStart = reinterpret_cast<const uint64_t *>(Data);
@@ -1311,7 +1317,7 @@ SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
if (std::error_code EC = Size.getError())
return EC;
- auto &TableVec = NameTable.setToEager();
+ std::vector<FunctionId> TableVec;
TableVec.reserve(*Size);
if (!ProfileIsCS)
MD5SampleContextTable.resize(*Size);
@@ -1325,6 +1331,8 @@ SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
}
if (!ProfileIsCS)
MD5SampleContextStart = MD5SampleContextTable.data();
+ NameTable =
+ std::make_unique<EagerSampleProfileNameTable>(std::move(TableVec));
return sampleprof_error::success;
}
More information about the llvm-commits
mailing list