Skip to content

Commit

Permalink
[clang][modules] Shrink the size of Module::Headers (llvm#113395)
Browse files Browse the repository at this point in the history
This patch shrinks the size of the `Module` class from 2112B to 1624B. I
wasn't able to get a good data on the actual impact on memory usage, but
given my `clang-scan-deps` workload at hand (with tens of thousands of
instances), I think there should be some win here. This also speeds up
my benchmark by under 0.1%.
  • Loading branch information
jansvoboda11 authored Oct 25, 2024
1 parent 4ac0e7e commit 6194668
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 36 deletions.
7 changes: 3 additions & 4 deletions clang-tools-extra/modularize/CoverageChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,9 @@ bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
return false;
}

for (auto &HeaderKind : Mod.Headers)
for (auto &Header : HeaderKind)
ModuleMapHeadersSet.insert(
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
for (const auto &Header : Mod.getAllHeaders())
ModuleMapHeadersSet.insert(
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));

for (auto *Submodule : Mod.submodules())
collectModuleHeaders(*Submodule);
Expand Down
14 changes: 3 additions & 11 deletions clang-tools-extra/modularize/ModularizeUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
} else if (std::optional<clang::Module::DirectoryName> UmbrellaDir =
Mod.getUmbrellaDirAsWritten()) {
// If there normal headers, assume these are umbrellas and skip collection.
if (Mod.Headers->size() == 0) {
if (Mod.getHeaders(Module::HK_Normal).empty()) {
// Collect headers in umbrella directory.
if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName(),
UmbrellaDependents))
Expand All @@ -371,16 +371,8 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
// modules or because they are meant to be included by another header,
// and thus should be ignored by modularize.

int NormalHeaderCount = Mod.Headers[clang::Module::HK_Normal].size();

for (int Index = 0; Index < NormalHeaderCount; ++Index) {
DependentsVector NormalDependents;
// Collect normal header.
const clang::Module::Header &Header(
Mod.Headers[clang::Module::HK_Normal][Index]);
std::string HeaderPath = getCanonicalPath(Header.Entry.getName());
HeaderFileNames.push_back(HeaderPath);
}
for (const auto &Header : Mod.getHeaders(clang::Module::HK_Normal))
HeaderFileNames.push_back(getCanonicalPath(Header.Entry.getName()));

int MissingCountThisModule = Mod.MissingHeaders.size();

Expand Down
31 changes: 24 additions & 7 deletions clang/include/clang/Basic/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,6 @@ class alignas(8) Module {
HK_PrivateTextual,
HK_Excluded
};
static const int NumHeaderKinds = HK_Excluded + 1;

/// Information about a header directive as found in the module map
/// file.
struct Header {
Expand All @@ -263,17 +261,36 @@ class alignas(8) Module {
FileEntryRef Entry;
};

/// Information about a directory name as found in the module map
/// file.
private:
static const int NumHeaderKinds = HK_Excluded + 1;
// The begin index for a HeaderKind also acts the end index of HeaderKind - 1.
// The extra element at the end acts as the end index of the last HeaderKind.
unsigned HeaderKindBeginIndex[NumHeaderKinds + 1] = {};
SmallVector<Header, 2> HeadersStorage;

public:
ArrayRef<Header> getAllHeaders() const { return HeadersStorage; }
ArrayRef<Header> getHeaders(HeaderKind HK) const {
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
auto BeginIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK];
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
return {BeginIt, EndIt};
}
void addHeader(HeaderKind HK, Header H) {
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
HeadersStorage.insert(EndIt, std::move(H));
for (unsigned HKI = HK + 1; HKI != NumHeaderKinds + 1; ++HKI)
++HeaderKindBeginIndex[HKI];
}

/// Information about a directory name as found in the module map file.
struct DirectoryName {
std::string NameAsWritten;
std::string PathRelativeToRootModuleDirectory;
DirectoryEntryRef Entry;
};

/// The headers that are part of this module.
SmallVector<Header, 2> Headers[5];

/// Stored information about a header directive that was found in the
/// module map file but has not been resolved to a file.
struct UnresolvedHeaderDirective {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Basic/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {

for (auto &K : Kinds) {
assert(&K == &Kinds[K.Kind] && "kinds in wrong order");
for (auto &H : Headers[K.Kind]) {
for (auto &H : getHeaders(K.Kind)) {
OS.indent(Indent + 2);
OS << K.Prefix << "header \"";
OS.write_escaped(H.NameAsWritten);
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ static std::error_code collectModuleHeaderIncludes(

// Add includes for each of these headers.
for (auto HK : {Module::HK_Normal, Module::HK_Private}) {
for (Module::Header &H : Module->Headers[HK]) {
for (const Module::Header &H : Module->getHeaders(HK)) {
Module->addTopHeader(H.Entry);
// Use the path as specified in the module map file. We'll look for this
// file relative to the module build directory (the directory containing
Expand Down
21 changes: 11 additions & 10 deletions clang/lib/Lex/ModuleMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ static bool violatesPrivateInclude(Module *RequestingModule,
// as obtained from the lookup and as obtained from the module.
// This check is not cheap, so enable it only for debugging.
bool IsPrivate = false;
SmallVectorImpl<Module::Header> *HeaderList[] = {
&Header.getModule()->Headers[Module::HK_Private],
&Header.getModule()->Headers[Module::HK_PrivateTextual]};
for (auto *Hs : HeaderList)
ArrayRef<Module::Header> HeaderList[] = {
Header.getModule()->getHeaders(Module::HK_Private),
Header.getModule()->getHeaders(Module::HK_PrivateTextual)};
for (auto Hs : HeaderList)
IsPrivate |= llvm::any_of(
*Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
assert(IsPrivate && "inconsistent headers and roles");
}
#endif
Expand Down Expand Up @@ -1296,27 +1296,28 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
ModuleHeaderRole Role, bool Imported) {
KnownHeader KH(Mod, Role);

FileEntryRef HeaderEntry = Header.Entry;

// Only add each header to the headers list once.
// FIXME: Should we diagnose if a header is listed twice in the
// same module definition?
auto &HeaderList = Headers[Header.Entry];
auto &HeaderList = Headers[HeaderEntry];
if (llvm::is_contained(HeaderList, KH))
return;

HeaderList.push_back(KH);
Mod->Headers[headerRoleToKind(Role)].push_back(Header);
Mod->addHeader(headerRoleToKind(Role), std::move(Header));

bool isCompilingModuleHeader = Mod->isForBuilding(LangOpts);
if (!Imported || isCompilingModuleHeader) {
// When we import HeaderFileInfo, the external source is expected to
// set the isModuleHeader flag itself.
HeaderInfo.MarkFileModuleHeader(Header.Entry, Role,
isCompilingModuleHeader);
HeaderInfo.MarkFileModuleHeader(HeaderEntry, Role, isCompilingModuleHeader);
}

// Notify callbacks that we just added a new header.
for (const auto &Cb : Callbacks)
Cb->moduleMapAddHeader(Header.Entry.getName());
Cb->moduleMapAddHeader(HeaderEntry.getName());
}

FileID ModuleMap::getContainingModuleMapFileID(const Module *Module) const {
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3070,9 +3070,9 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Module::HK_PrivateTextual},
{SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Module::HK_Excluded}
};
for (auto &HL : HeaderLists) {
for (const auto &HL : HeaderLists) {
RecordData::value_type Record[] = {HL.RecordKind};
for (auto &H : Mod->Headers[HL.HeaderKind])
for (const auto &H : Mod->getHeaders(HL.HeaderKind))
Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
}

Expand Down

0 comments on commit 6194668

Please sign in to comment.