Skip to content

[WIP][C++20][Modules] Lazily, but fully load 'HeaderFileInfo' table into memory. #140867

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
4 changes: 4 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,10 @@ class ASTReader
/// Map from the TU to its lexical contents from each module file.
std::vector<std::pair<ModuleFile*, LexicalContents>> TULexicalDecls;

unsigned HeaderFileInfoIdx = 0;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Unloaded header file info idx.

And also I feel the name is odd. It is index for module files, not header file infos.

Copy link
Member

Choose a reason for hiding this comment

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

And also, this seems unsafe since ModuleManager can remove modules technically. Maybe it is better to insert a bool in ModuleFile to mark if its header info is loaded.


llvm::DenseMap<FileEntryRef, HeaderFileInfo> HeaderFileInfoLookup;

/// Map from a DeclContext to its lookup tables.
llvm::DenseMap<const DeclContext *,
serialization::reader::DeclContextLookupTable> Lookups;
Expand Down
80 changes: 49 additions & 31 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4155,6 +4155,7 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
if (Record[0]) {
F.HeaderFileInfoTable = HeaderFileInfoLookupTable::Create(
(const unsigned char *)F.HeaderFileInfoTableData + Record[0],
(const unsigned char *)F.HeaderFileInfoTableData + sizeof(uint32_t),
(const unsigned char *)F.HeaderFileInfoTableData,
HeaderFileInfoTrait(*this, F));

Expand Down Expand Up @@ -6831,43 +6832,60 @@ std::optional<bool> ASTReader::isPreprocessedEntityInFileID(unsigned Index,
return false;
}

namespace {

/// Visitor used to search for information about a header file.
class HeaderFileInfoVisitor {
FileEntryRef FE;
std::optional<HeaderFileInfo> HFI;

public:
explicit HeaderFileInfoVisitor(FileEntryRef FE) : FE(FE) {}

bool operator()(ModuleFile &M) {
HeaderFileInfoLookupTable *Table
= static_cast<HeaderFileInfoLookupTable *>(M.HeaderFileInfoTable);
if (!Table)
return false;
static void mergeHeaderFileInfoModuleBits(HeaderFileInfo &HFI,
bool isModuleHeader,
bool isTextualModuleHeader) {
HFI.isModuleHeader |= isModuleHeader;
if (HFI.isModuleHeader)
HFI.isTextualModuleHeader = false;
else
HFI.isTextualModuleHeader |= isTextualModuleHeader;
}

// Look in the on-disk hash table for an entry for this file name.
HeaderFileInfoLookupTable::iterator Pos = Table->find(FE);
if (Pos == Table->end())
return false;
/// Merge the header file info provided by \p OtherHFI into the current
/// header file info (\p HFI)
static void mergeHeaderFileInfo(HeaderFileInfo &HFI,
const HeaderFileInfo &OtherHFI) {
assert(OtherHFI.External && "expected to merge external HFI");

HFI = *Pos;
return true;
}
HFI.isImport |= OtherHFI.isImport;
HFI.isPragmaOnce |= OtherHFI.isPragmaOnce;
mergeHeaderFileInfoModuleBits(HFI, OtherHFI.isModuleHeader,
OtherHFI.isTextualModuleHeader);

std::optional<HeaderFileInfo> getHeaderFileInfo() const { return HFI; }
};
if (!HFI.LazyControllingMacro.isValid())
HFI.LazyControllingMacro = OtherHFI.LazyControllingMacro;

} // namespace
HFI.DirInfo = OtherHFI.DirInfo;
HFI.External = (!HFI.IsValid || HFI.External);
HFI.IsValid = true;
}

HeaderFileInfo ASTReader::GetHeaderFileInfo(FileEntryRef FE) {
HeaderFileInfoVisitor Visitor(FE);
ModuleMgr.visit(Visitor);
if (std::optional<HeaderFileInfo> HFI = Visitor.getHeaderFileInfo())
return *HFI;

return HeaderFileInfo();
for (auto Iter = ModuleMgr.begin() + HeaderFileInfoIdx, End = ModuleMgr.end();
Iter != End; ++Iter) {
if (auto *Table = static_cast<HeaderFileInfoLookupTable *>(
Iter->HeaderFileInfoTable)) {
auto &Info = Table->getInfoObj();
for (auto Iter = Table->data_begin(), End = Table->data_end();
Iter != End; ++Iter) {
const auto *Item = Iter.getItem();
// Determine the length of the key and the data.
const auto &[KeyLen, DataLen] =
HeaderFileInfoTrait::ReadKeyDataLength(Item);
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe it is better to wrap such logics in ASTReaderInternals.h

// Read the key.
const auto &Key = Info.ReadKey(Item, KeyLen);
if (auto EKey = Info.getFile(Key)) {
auto Data = Info.ReadData(Key, Item + KeyLen, DataLen);
auto [Iter, Inserted] = HeaderFileInfoLookup.try_emplace(*EKey, Data);
if (!Inserted)
mergeHeaderFileInfo(Iter->second, Data);
}
}
}
}
HeaderFileInfoIdx = ModuleMgr.size();
return HeaderFileInfoLookup.lookup(FE);
}

void ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &Diag) {
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Serialization/ASTReaderInternals.h
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,12 @@ class HeaderFileInfoTrait {

data_type ReadData(internal_key_ref,const unsigned char *d, unsigned DataLen);

private:
OptionalFileEntryRef getFile(const internal_key_type &Key);
};

/// The on-disk hash table used for known header files.
using HeaderFileInfoLookupTable =
llvm::OnDiskChainedHashTable<HeaderFileInfoTrait>;
llvm::OnDiskIterableChainedHashTable<HeaderFileInfoTrait>;

} // namespace reader

Expand Down
Loading