Skip to content

[CGData] Lazy loading support for stable function map #151660

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions llvm/include/llvm/CGData/CodeGenData.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ enum CGDataVersion {
// Version 3 adds the total size of the Names in the stable function map so
// we can skip reading them into the memory for non-assertion builds.
Version3 = 3,
// Version 4 adjusts the structure of stable function merging map for
// efficient lazy loading support.
Version4 = 4,
CurrentVersion = CG_DATA_INDEX_VERSION
};
const uint64_t Version = CGDataVersion::CurrentVersion;
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/CGData/CodeGenData.inc
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ CG_DATA_SECT_ENTRY(CG_merge, CG_DATA_QUOTE(CG_DATA_MERGE_COMMON),
#endif

/* Indexed codegen data format version (start from 1). */
#define CG_DATA_INDEX_VERSION 3
#define CG_DATA_INDEX_VERSION 4
51 changes: 46 additions & 5 deletions llvm/include/llvm/CGData/StableFunctionMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/IR/StructuralHash.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/MemoryBuffer.h"
#include <mutex>

namespace llvm {

Expand Down Expand Up @@ -72,11 +74,29 @@ struct StableFunctionMap {
IndexOperandHashMap(std::move(IndexOperandHashMap)) {}
};

using HashFuncsMapType =
DenseMap<stable_hash, SmallVector<std::unique_ptr<StableFunctionEntry>>>;
using StableFunctionEntries =
SmallVector<std::unique_ptr<StableFunctionEntry>>;

/// In addition to the deserialized StableFunctionEntry, the struct stores
/// the offsets of corresponding serialized stable function entries, and a
/// once flag for safe lazy loading in a multithreaded environment.
struct EntryStorage {
StableFunctionEntries Entries;

private:
SmallVector<uint64_t> Offsets;
std::once_flag LazyLoadFlag;
friend struct StableFunctionMap;
friend struct StableFunctionMapRecord;
};

// Note: DenseMap requires value type to be copyable even if only using
// in-place insertion. Use STL instead. This also affects the
// deletion-while-iteration in finalize().
using HashFuncsMapType = std::unordered_map<stable_hash, EntryStorage>;

/// Get the HashToFuncs map for serialization.
const HashFuncsMapType &getFunctionMap() const { return HashToFuncs; }
const HashFuncsMapType &getFunctionMap() const;

/// Get the NameToId vector for serialization.
ArrayRef<std::string> getNames() const { return IdToName; }
Expand All @@ -99,6 +119,13 @@ struct StableFunctionMap {
/// \returns true if there is no stable function entry.
bool empty() const { return size() == 0; }

bool contains(HashFuncsMapType::key_type FunctionHash) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment with ///.

return HashToFuncs.count(FunctionHash) > 0;
}

const StableFunctionEntries &
at(HashFuncsMapType::key_type FunctionHash) const;

enum SizeType {
UniqueHashCount, // The number of unique hashes in HashToFuncs.
TotalFunctionCount, // The number of total functions in HashToFuncs.
Expand All @@ -119,17 +146,31 @@ struct StableFunctionMap {
/// `StableFunctionEntry` is ready for insertion.
void insert(std::unique_ptr<StableFunctionEntry> FuncEntry) {
assert(!Finalized && "Cannot insert after finalization");
HashToFuncs[FuncEntry->Hash].emplace_back(std::move(FuncEntry));
HashToFuncs[FuncEntry->Hash].Entries.emplace_back(std::move(FuncEntry));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify what happens to Offsets of EntryStorage? It appears they've already been populated. You might also comment when Entries and Offsets are used/written in EntryStorage type above.

}

void deserializeLazyLoadingEntry(HashFuncsMapType::iterator It);

/// Eagerly deserialize all the unloaded entries in the lazy loading map.
void deserializeLazyLoadingEntries();

bool isLazilyLoaded() const { return (bool)Buffer; }

/// A map from a stable_hash to a vector of functions with that hash.
HashFuncsMapType HashToFuncs;
mutable HashFuncsMapType HashToFuncs;
/// A vector of strings to hold names.
SmallVector<std::string> IdToName;
/// A map from StringRef (name) to an ID.
StringMap<unsigned> NameToId;
/// True if the function map is finalized with minimal content.
bool Finalized = false;
/// The memory buffer that contains the serialized stable function map for
/// lazy loading.
/// Non-empty only if this StableFunctionMap is created from a MemoryBuffer
/// (i.e. by IndexedCodeGenDataReader::read()) and lazily deserialized.
std::shared_ptr<MemoryBuffer> Buffer;
/// Whether to read stable function names from the buffer.
bool ReadStableFunctionMapNames = true;

friend struct StableFunctionMapRecord;
};
Expand Down
19 changes: 19 additions & 0 deletions llvm/include/llvm/CGData/StableFunctionMapRecord.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ struct StableFunctionMapRecord {
const StableFunctionMap *FunctionMap,
std::vector<CGDataPatchItem> &PatchItems);

/// A static helper function to deserialize the stable function map entry.
/// Ptr should be pointing to the start of the fixed-sized fields of the
/// entry when passed in.
LLVM_ABI static void deserializeEntry(const unsigned char *Ptr,
stable_hash Hash,
StableFunctionMap *FunctionMap,
bool ReadStableFunctionMapNames = true);

/// Serialize the stable function map to a raw_ostream.
LLVM_ABI void serialize(raw_ostream &OS,
std::vector<CGDataPatchItem> &PatchItems) const;
Expand All @@ -48,6 +56,13 @@ struct StableFunctionMapRecord {
LLVM_ABI void deserialize(const unsigned char *&Ptr,
bool ReadStableFunctionMapNames = true);

/// Lazily deserialize the stable function map from `Buffer` starting at
/// `Offset`. The individial stable function entry would be read lazily from
/// `Buffer` when the function map is accessed.
LLVM_ABI void lazyDeserialize(std::shared_ptr<MemoryBuffer> Buffer,
uint64_t Offset,
bool ReadStableFunctionMapNames = true);

/// Serialize the stable function map to a YAML stream.
LLVM_ABI void serializeYAML(yaml::Output &YOS) const;

Expand All @@ -70,6 +85,10 @@ struct StableFunctionMapRecord {
yaml::Output YOS(OS);
serializeYAML(YOS);
}

private:
void deserialize(const unsigned char *&Ptr, bool ReadStableFunctionMapNames,
bool Lazy);
};

} // namespace llvm
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CGData/CodeGenData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ Expected<Header> Header::readFromBuffer(const unsigned char *Curr) {
return make_error<CGDataError>(cgdata_error::unsupported_version);
H.DataKind = endian::readNext<uint32_t, endianness::little, unaligned>(Curr);

static_assert(IndexedCGData::CGDataVersion::CurrentVersion == Version3,
static_assert(IndexedCGData::CGDataVersion::CurrentVersion == Version4,
"Please update the offset computation below if a new field has "
"been added to the header.");
H.OutlinedHashTreeOffset =
Expand Down
17 changes: 16 additions & 1 deletion llvm/lib/CGData/CodeGenDataReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ static cl::opt<bool> IndexedCodeGenDataReadFunctionMapNames(
"disabled to save memory and time for final consumption of the "
"indexed CodeGenData in production."));

cl::opt<bool> IndexedCodeGenDataLazyLoading(
"indexed-codegen-data-lazy-loading", cl::init(false), cl::Hidden,
cl::desc(
"Lazily load indexed CodeGenData. Enable to save memory and time "
"for final consumption of the indexed CodeGenData in production."));

namespace llvm {

static Expected<std::unique_ptr<MemoryBuffer>>
Expand Down Expand Up @@ -109,11 +115,20 @@ Error IndexedCodeGenDataReader::read() {
return error(cgdata_error::eof);
HashTreeRecord.deserialize(Ptr);
}

// TODO: lazy loading support for outlined hash tree.
std::shared_ptr<MemoryBuffer> SharedDataBuffer = std::move(DataBuffer);
if (hasStableFunctionMap()) {
const unsigned char *Ptr = Start + Header.StableFunctionMapOffset;
if (Ptr >= End)
return error(cgdata_error::eof);
FunctionMapRecord.deserialize(Ptr, IndexedCodeGenDataReadFunctionMapNames);
if (IndexedCodeGenDataLazyLoading)
FunctionMapRecord.lazyDeserialize(SharedDataBuffer,
Header.StableFunctionMapOffset,
IndexedCodeGenDataReadFunctionMapNames);
Copy link
Contributor

Choose a reason for hiding this comment

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

Now, you set IndexedCodeGenDataReadFunctionMapNames in FunctionMap. Can we set it (perhaps using an explicit function) before if/else? Then you don't need to pass around IndexedCodeGenDataReadFunctionMapNames arguments across many functions as you can just refer to it from the FunctionMap.

else
FunctionMapRecord.deserialize(Ptr,
IndexedCodeGenDataReadFunctionMapNames);
}

return success();
Expand Down
68 changes: 55 additions & 13 deletions llvm/lib/CGData/StableFunctionMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@

#include "llvm/CGData/StableFunctionMap.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/CGData/StableFunctionMapRecord.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include <mutex>

#define DEBUG_TYPE "stable-function-map"

Expand Down Expand Up @@ -93,9 +95,10 @@ void StableFunctionMap::insert(const StableFunction &Func) {

void StableFunctionMap::merge(const StableFunctionMap &OtherMap) {
assert(!Finalized && "Cannot merge after finalization");
deserializeLazyLoadingEntries();
for (auto &[Hash, Funcs] : OtherMap.HashToFuncs) {
auto &ThisFuncs = HashToFuncs[Hash];
for (auto &Func : Funcs) {
auto &ThisFuncs = HashToFuncs[Hash].Entries;
for (auto &Func : Funcs.Entries) {
auto FuncNameId =
getIdOrCreateForName(*OtherMap.getNameForId(Func->FunctionNameId));
auto ModuleNameId =
Expand All @@ -114,25 +117,61 @@ size_t StableFunctionMap::size(SizeType Type) const {
case UniqueHashCount:
return HashToFuncs.size();
case TotalFunctionCount: {
const_cast<StableFunctionMap *>(this)->deserializeLazyLoadingEntries();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to make deserializeLazyLoadingEntries() const?

size_t Count = 0;
for (auto &Funcs : HashToFuncs)
Count += Funcs.second.size();
Count += Funcs.second.Entries.size();
return Count;
}
case MergeableFunctionCount: {
const_cast<StableFunctionMap *>(this)->deserializeLazyLoadingEntries();
size_t Count = 0;
for (auto &[Hash, Funcs] : HashToFuncs)
if (Funcs.size() >= 2)
Count += Funcs.size();
if (Funcs.Entries.size() >= 2)
Count += Funcs.Entries.size();
return Count;
}
}
llvm_unreachable("Unhandled size type");
}

const StableFunctionMap::StableFunctionEntries &
StableFunctionMap::at(HashFuncsMapType::key_type FunctionHash) const {
auto It = HashToFuncs.find(FunctionHash);
if (isLazilyLoaded())
const_cast<StableFunctionMap *>(this)->deserializeLazyLoadingEntry(It);
return It->second.Entries;
}

void StableFunctionMap::deserializeLazyLoadingEntry(
HashFuncsMapType::iterator It) {
assert(isLazilyLoaded() && "Cannot deserialize non-lazily-loaded map");
std::call_once(It->second.LazyLoadFlag, [this, It]() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this more explicit for the better readability?

Suggested change
std::call_once(It->second.LazyLoadFlag, [this, It]() {
const auto& [Hash, Storage] = *It;
std::call_once(Storage.LazyLoadFlag, [this, &Hash, &Storage]() {

Copy link
Contributor

Choose a reason for hiding this comment

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

I've encountered a weird "bug" where "Structured bindings cannot be captured by lambda expressions" until C++20. A workaround is to rename the captured variable

https://en.cppreference.com/w/cpp/language/structured_binding.html

Suggested change
std::call_once(It->second.LazyLoadFlag, [this, It]() {
const auto& [Hash, Storage] = *It;
std::call_once(It->second.LazyLoadFlag, [this, HashArg = Hash, StorageArg = Storage]() {

for (auto Offset : It->second.Offsets)
StableFunctionMapRecord::deserializeEntry(
reinterpret_cast<const unsigned char *>(Offset), It->first, this,
ReadStableFunctionMapNames);
});
}

void ::StableFunctionMap::deserializeLazyLoadingEntries() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void ::StableFunctionMap::deserializeLazyLoadingEntries() {
void StableFunctionMap::deserializeLazyLoadingEntries() {

if (!isLazilyLoaded())
return;
for (auto It = HashToFuncs.begin(); It != HashToFuncs.end(); ++It)
deserializeLazyLoadingEntry(It);
}

const StableFunctionMap::HashFuncsMapType &
StableFunctionMap::getFunctionMap() const {
// Ensure all entries are deserialized before returning the raw map.
if (isLazilyLoaded())
const_cast<StableFunctionMap *>(this)->deserializeLazyLoadingEntries();
return HashToFuncs;
}

using ParamLocs = SmallVector<IndexPair>;
static void removeIdenticalIndexPair(
SmallVector<std::unique_ptr<StableFunctionMap::StableFunctionEntry>> &SFS) {
static void
removeIdenticalIndexPair(StableFunctionMap::StableFunctionEntries &SFS) {
auto &RSF = SFS[0];
unsigned StableFunctionCount = SFS.size();

Expand All @@ -159,9 +198,7 @@ static void removeIdenticalIndexPair(
SF->IndexOperandHashMap->erase(Pair);
}

static bool isProfitable(
const SmallVector<std::unique_ptr<StableFunctionMap::StableFunctionEntry>>
&SFS) {
static bool isProfitable(const StableFunctionMap::StableFunctionEntries &SFS) {
unsigned StableFunctionCount = SFS.size();
if (StableFunctionCount < GlobalMergingMinMerges)
return false;
Expand Down Expand Up @@ -202,8 +239,11 @@ static bool isProfitable(
}

void StableFunctionMap::finalize(bool SkipTrim) {
deserializeLazyLoadingEntries();
SmallVector<HashFuncsMapType::iterator> ToDelete;
for (auto It = HashToFuncs.begin(); It != HashToFuncs.end(); ++It) {
auto &[StableHash, SFS] = *It;
auto &[StableHash, Storage] = *It;
auto &SFS = Storage.Entries;

// Group stable functions by ModuleIdentifier.
llvm::stable_sort(SFS, [&](const std::unique_ptr<StableFunctionEntry> &L,
Expand Down Expand Up @@ -236,7 +276,7 @@ void StableFunctionMap::finalize(bool SkipTrim) {
}
}
if (Invalid) {
HashToFuncs.erase(It);
ToDelete.push_back(It);
continue;
}

Expand All @@ -248,8 +288,10 @@ void StableFunctionMap::finalize(bool SkipTrim) {
removeIdenticalIndexPair(SFS);

if (!isProfitable(SFS))
HashToFuncs.erase(It);
ToDelete.push_back(It);
}
for (auto It : ToDelete)
HashToFuncs.erase(It);

Finalized = true;
}
Loading