Skip to content

Commit 4d006ca

Browse files
committed
[clang][modules] Shrink the size of Module::Headers (llvm#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%. (cherry picked from commit 6194668)
1 parent ffe7991 commit 4d006ca

File tree

7 files changed

+45
-36
lines changed

7 files changed

+45
-36
lines changed

clang-tools-extra/modularize/CoverageChecker.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,9 @@ bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
223223
return false;
224224
}
225225

226-
for (auto &HeaderKind : Mod.Headers)
227-
for (auto &Header : HeaderKind)
228-
ModuleMapHeadersSet.insert(
229-
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
226+
for (const auto &Header : Mod.getAllHeaders())
227+
ModuleMapHeadersSet.insert(
228+
ModularizeUtilities::getCanonicalPath(Header.Entry.getName()));
230229

231230
for (auto *Submodule : Mod.submodules())
232231
collectModuleHeaders(*Submodule);

clang-tools-extra/modularize/ModularizeUtilities.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
358358
} else if (std::optional<clang::Module::DirectoryName> UmbrellaDir =
359359
Mod.getUmbrellaDirAsWritten()) {
360360
// If there normal headers, assume these are umbrellas and skip collection.
361-
if (Mod.Headers->size() == 0) {
361+
if (Mod.getHeaders(Module::HK_Normal).empty()) {
362362
// Collect headers in umbrella directory.
363363
if (!collectUmbrellaHeaders(UmbrellaDir->Entry.getName(),
364364
UmbrellaDependents))
@@ -371,16 +371,8 @@ bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
371371
// modules or because they are meant to be included by another header,
372372
// and thus should be ignored by modularize.
373373

374-
int NormalHeaderCount = Mod.Headers[clang::Module::HK_Normal].size();
375-
376-
for (int Index = 0; Index < NormalHeaderCount; ++Index) {
377-
DependentsVector NormalDependents;
378-
// Collect normal header.
379-
const clang::Module::Header &Header(
380-
Mod.Headers[clang::Module::HK_Normal][Index]);
381-
std::string HeaderPath = getCanonicalPath(Header.Entry.getName());
382-
HeaderFileNames.push_back(HeaderPath);
383-
}
374+
for (const auto &Header : Mod.getHeaders(clang::Module::HK_Normal))
375+
HeaderFileNames.push_back(getCanonicalPath(Header.Entry.getName()));
384376

385377
int MissingCountThisModule = Mod.MissingHeaders.size();
386378

clang/include/clang/Basic/Module.h

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ class alignas(8) Module {
262262
HK_PrivateTextual,
263263
HK_Excluded
264264
};
265-
static const int NumHeaderKinds = HK_Excluded + 1;
266-
267265
/// Information about a header directive as found in the module map
268266
/// file.
269267
struct Header {
@@ -272,17 +270,36 @@ class alignas(8) Module {
272270
FileEntryRef Entry;
273271
};
274272

275-
/// Information about a directory name as found in the module map
276-
/// file.
273+
private:
274+
static const int NumHeaderKinds = HK_Excluded + 1;
275+
// The begin index for a HeaderKind also acts the end index of HeaderKind - 1.
276+
// The extra element at the end acts as the end index of the last HeaderKind.
277+
unsigned HeaderKindBeginIndex[NumHeaderKinds + 1] = {};
278+
SmallVector<Header, 2> HeadersStorage;
279+
280+
public:
281+
ArrayRef<Header> getAllHeaders() const { return HeadersStorage; }
282+
ArrayRef<Header> getHeaders(HeaderKind HK) const {
283+
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
284+
auto BeginIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK];
285+
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
286+
return {BeginIt, EndIt};
287+
}
288+
void addHeader(HeaderKind HK, Header H) {
289+
assert(HK < NumHeaderKinds && "Invalid Module::HeaderKind");
290+
auto EndIt = HeadersStorage.begin() + HeaderKindBeginIndex[HK + 1];
291+
HeadersStorage.insert(EndIt, std::move(H));
292+
for (unsigned HKI = HK + 1; HKI != NumHeaderKinds + 1; ++HKI)
293+
++HeaderKindBeginIndex[HKI];
294+
}
295+
296+
/// Information about a directory name as found in the module map file.
277297
struct DirectoryName {
278298
std::string NameAsWritten;
279299
std::string PathRelativeToRootModuleDirectory;
280300
DirectoryEntryRef Entry;
281301
};
282302

283-
/// The headers that are part of this module.
284-
SmallVector<Header, 2> Headers[5];
285-
286303
/// Stored information about a header directive that was found in the
287304
/// module map file but has not been resolved to a file.
288305
struct UnresolvedHeaderDirective {

clang/lib/Basic/Module.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const {
537537

538538
for (auto &K : Kinds) {
539539
assert(&K == &Kinds[K.Kind] && "kinds in wrong order");
540-
for (auto &H : Headers[K.Kind]) {
540+
for (auto &H : getHeaders(K.Kind)) {
541541
OS.indent(Indent + 2);
542542
OS << K.Prefix << "header \"";
543543
OS.write_escaped(H.NameAsWritten);

clang/lib/Frontend/FrontendAction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ static std::error_code collectModuleHeaderIncludes(
367367

368368
// Add includes for each of these headers.
369369
for (auto HK : {Module::HK_Normal, Module::HK_Private}) {
370-
for (Module::Header &H : Module->Headers[HK]) {
370+
for (const Module::Header &H : Module->getHeaders(HK)) {
371371
Module->addTopHeader(H.Entry);
372372
// Use the path as specified in the module map file. We'll look for this
373373
// file relative to the module build directory (the directory containing

clang/lib/Lex/ModuleMap.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,12 @@ static bool violatesPrivateInclude(Module *RequestingModule,
472472
// as obtained from the lookup and as obtained from the module.
473473
// This check is not cheap, so enable it only for debugging.
474474
bool IsPrivate = false;
475-
SmallVectorImpl<Module::Header> *HeaderList[] = {
476-
&Header.getModule()->Headers[Module::HK_Private],
477-
&Header.getModule()->Headers[Module::HK_PrivateTextual]};
478-
for (auto *Hs : HeaderList)
475+
ArrayRef<Module::Header> HeaderList[] = {
476+
Header.getModule()->getHeaders(Module::HK_Private),
477+
Header.getModule()->getHeaders(Module::HK_PrivateTextual)};
478+
for (auto Hs : HeaderList)
479479
IsPrivate |= llvm::any_of(
480-
*Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
480+
Hs, [&](const Module::Header &H) { return H.Entry == IncFileEnt; });
481481
assert(IsPrivate && "inconsistent headers and roles");
482482
}
483483
#endif
@@ -1304,27 +1304,28 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header,
13041304
ModuleHeaderRole Role, bool Imported) {
13051305
KnownHeader KH(Mod, Role);
13061306

1307+
FileEntryRef HeaderEntry = Header.Entry;
1308+
13071309
// Only add each header to the headers list once.
13081310
// FIXME: Should we diagnose if a header is listed twice in the
13091311
// same module definition?
1310-
auto &HeaderList = Headers[Header.Entry];
1312+
auto &HeaderList = Headers[HeaderEntry];
13111313
if (llvm::is_contained(HeaderList, KH))
13121314
return;
13131315

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

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

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

13301331
FileID ModuleMap::getContainingModuleMapFileID(const Module *Module) const {

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3089,9 +3089,9 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
30893089
Module::HK_PrivateTextual},
30903090
{SUBMODULE_EXCLUDED_HEADER, ExcludedHeaderAbbrev, Module::HK_Excluded}
30913091
};
3092-
for (auto &HL : HeaderLists) {
3092+
for (const auto &HL : HeaderLists) {
30933093
RecordData::value_type Record[] = {HL.RecordKind};
3094-
for (auto &H : Mod->Headers[HL.HeaderKind])
3094+
for (const auto &H : Mod->getHeaders(HL.HeaderKind))
30953095
Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
30963096
}
30973097

0 commit comments

Comments
 (0)