https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/113395
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%. >From 09246d11c8663c0b2b31317eddc297c1d29fcd60 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <jan_svob...@apple.com> Date: Tue, 22 Oct 2024 16:07:27 -0700 Subject: [PATCH] [clang][modules] Shrink the size of `Module::Headers` 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%. --- clang/include/clang/Basic/Module.h | 18 ++++++++++++++++-- clang/lib/Basic/Module.cpp | 2 +- clang/lib/Frontend/FrontendAction.cpp | 2 +- clang/lib/Lex/ModuleMap.cpp | 20 +++++++++++--------- clang/lib/Serialization/ASTWriter.cpp | 4 ++-- 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 9c5d33fbb562cc..b8f1e00d6fac1f 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -271,8 +271,22 @@ class alignas(8) Module { DirectoryEntryRef Entry; }; - /// The headers that are part of this module. - SmallVector<Header, 2> Headers[5]; +private: + unsigned HeaderKindBeginIndex[6] = {}; + SmallVector<Header, 2> HeadersStorage; + +public: + ArrayRef<Header> getHeaders(HeaderKind HK) const { + auto BeginIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK]; + auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1]; + return {BeginIt, EndIt}; + } + void addHeader(HeaderKind HK, Header H) { + auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1]; + HeadersStorage.insert(EndIt, std::move(H)); + for (unsigned HKI = HK + 1; HKI != 6; ++HKI) + ++HeaderKindBeginIndex[HKI]; + } /// Stored information about a header directive that was found in the /// module map file but has not been resolved to a file. diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index ad52fccff5dc7f..a7a3f6b37efef1 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -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); diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 81eea9c4c4dc58..8264bd702fe43f 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -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 diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index fd6bc17ae9cdac..23875912fa735e 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -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 @@ -1296,27 +1296,29 @@ 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, + 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 { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 494890284d2f2c..b576822fa704c8 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -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); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits