Skip to content

Commit eedbf7c

Browse files
committed
[clang][modules] Optimize construction and usage of the submodule index (llvm#113391)
This patch avoids eagerly populating the submodule index on `Module` construction. The `StringMap` allocation shows up in my profiles of `clang-scan-deps`, while the index is not necessary most of the time. We still construct it on-demand. Moreover, this patch avoids performing qualified submodule lookup in `ASTReader` whenever we're serializing a module graph whose top-level module is unknown. This is pointless, since that's guaranteed to never find any existing submodules anyway. This speeds up `clang-scan-deps` by ~0.5% on my workload. (cherry picked from commit 6c6351e)
1 parent ab6adca commit eedbf7c

File tree

5 files changed

+42
-29
lines changed

5 files changed

+42
-29
lines changed

clang/include/clang/Basic/Module.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ class alignas(8) Module {
233233

234234
/// A mapping from the submodule name to the index into the
235235
/// \c SubModules vector at which that submodule resides.
236-
llvm::StringMap<unsigned> SubModuleIndex;
236+
mutable llvm::StringMap<unsigned> SubModuleIndex;
237237

238238
/// The AST file if this is a top-level module which has a
239239
/// corresponding serialized AST file, or null otherwise.
@@ -629,7 +629,6 @@ class alignas(8) Module {
629629
void setParent(Module *M) {
630630
assert(!Parent);
631631
Parent = M;
632-
Parent->SubModuleIndex[Name] = Parent->SubModules.size();
633632
Parent->SubModules.push_back(this);
634633
}
635634

clang/include/clang/Lex/ModuleMap.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -552,11 +552,14 @@ class ModuleMap {
552552
///
553553
/// \param IsExplicit Whether this is an explicit submodule.
554554
///
555-
/// \returns The found or newly-created module, along with a boolean value
556-
/// that will be true if the module is newly-created.
557-
std::pair<Module *, bool> findOrCreateModule(StringRef Name, Module *Parent,
558-
bool IsFramework,
559-
bool IsExplicit);
555+
/// \returns The found or newly-created module.
556+
Module *findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
557+
bool IsExplicit);
558+
/// Create new submodule, assuming it does not exist. This function can only
559+
/// be called when it is guaranteed that this submodule does not exist yet.
560+
/// The parameters have same semantics as \c ModuleMap::findOrCreateModule.
561+
Module *createModule(StringRef Name, Module *Parent, bool IsFramework,
562+
bool IsExplicit);
560563

561564
/// Create a global module fragment for a C++ module unit.
562565
///

clang/lib/Basic/Module.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ Module::Module(ModuleConstructorTag, StringRef Name,
6060
NoUndeclaredIncludes = Parent->NoUndeclaredIncludes;
6161
ModuleMapIsPrivate = Parent->ModuleMapIsPrivate;
6262

63-
Parent->SubModuleIndex[Name] = Parent->SubModules.size();
6463
Parent->SubModules.push_back(this);
6564
}
6665
}
@@ -358,11 +357,14 @@ void Module::markUnavailable(bool Unimportable) {
358357
}
359358

360359
Module *Module::findSubmodule(StringRef Name) const {
361-
llvm::StringMap<unsigned>::const_iterator Pos = SubModuleIndex.find(Name);
362-
if (Pos == SubModuleIndex.end())
363-
return nullptr;
360+
// Add new submodules into the index.
361+
for (unsigned I = SubModuleIndex.size(), E = SubModules.size(); I != E; ++I)
362+
SubModuleIndex[SubModules[I]->Name] = I;
364363

365-
return SubModules[Pos->getValue()];
364+
if (auto It = SubModuleIndex.find(Name); It != SubModuleIndex.end())
365+
return SubModules[It->second];
366+
367+
return nullptr;
366368
}
367369

368370
Module *Module::getGlobalModuleFragment() const {

clang/lib/Lex/ModuleMap.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -655,8 +655,8 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
655655
SmallString<32> NameBuf;
656656
StringRef Name = sanitizeFilenameAsIdentifier(
657657
llvm::sys::path::stem(SkippedDir.getName()), NameBuf);
658-
Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
659-
Explicit).first;
658+
Result =
659+
findOrCreateModule(Name, Result, /*IsFramework=*/false, Explicit);
660660
setInferredModuleAllowedBy(Result, UmbrellaModuleMap);
661661

662662
// Associate the module and the directory.
@@ -672,8 +672,8 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
672672
SmallString<32> NameBuf;
673673
StringRef Name = sanitizeFilenameAsIdentifier(
674674
llvm::sys::path::stem(File.getName()), NameBuf);
675-
Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
676-
Explicit).first;
675+
Result =
676+
findOrCreateModule(Name, Result, /*IsFramework=*/false, Explicit);
677677
setInferredModuleAllowedBy(Result, UmbrellaModuleMap);
678678
Result->addTopHeader(File);
679679

@@ -866,15 +866,21 @@ Module *ModuleMap::lookupModuleQualified(StringRef Name, Module *Context) const{
866866
return Context->findSubmodule(Name);
867867
}
868868

869-
std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
870-
Module *Parent,
871-
bool IsFramework,
872-
bool IsExplicit) {
869+
Module *ModuleMap::findOrCreateModule(StringRef Name, Module *Parent,
870+
bool IsFramework, bool IsExplicit) {
873871
// Try to find an existing module with this name.
874872
if (Module *Sub = lookupModuleQualified(Name, Parent))
875-
return std::make_pair(Sub, false);
873+
return Sub;
876874

877875
// Create a new module with this name.
876+
return createModule(Name, Parent, IsFramework, IsExplicit);
877+
}
878+
879+
Module *ModuleMap::createModule(StringRef Name, Module *Parent,
880+
bool IsFramework, bool IsExplicit) {
881+
assert(lookupModuleQualified(Name, Parent) == nullptr &&
882+
"Creating duplicate submodule");
883+
878884
Module *Result = new (ModulesAlloc.Allocate())
879885
Module(ModuleConstructorTag{}, Name, SourceLocation(), Parent,
880886
IsFramework, IsExplicit, NumCreatedModules++);
@@ -884,7 +890,7 @@ std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
884890
Modules[Name] = Result;
885891
ModuleScopeIDs[Result] = CurrentModuleScopeID;
886892
}
887-
return std::make_pair(Result, true);
893+
return Result;
888894
}
889895

890896
Module *ModuleMap::createGlobalModuleFragmentForModuleUnit(SourceLocation Loc,
@@ -2135,8 +2141,7 @@ void ModuleMapParser::parseModuleDecl() {
21352141
Map.createShadowedModule(ModuleName, Framework, ShadowingModule);
21362142
} else {
21372143
ActiveModule =
2138-
Map.findOrCreateModule(ModuleName, ActiveModule, Framework, Explicit)
2139-
.first;
2144+
Map.findOrCreateModule(ModuleName, ActiveModule, Framework, Explicit);
21402145
}
21412146

21422147
ActiveModule->DefinitionLoc = ModuleNameLoc;

clang/lib/Serialization/ASTReader.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5756,6 +5756,13 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
57565756
return Err;
57575757

57585758
ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap();
5759+
bool KnowsTopLevelModule = ModMap.findModule(F.ModuleName) != nullptr;
5760+
// If we don't know the top-level module, there's no point in doing qualified
5761+
// lookup of its submodules; it won't find anything anywhere within this tree.
5762+
// Let's skip that and avoid some string lookups.
5763+
auto CreateModule = !KnowsTopLevelModule ? &ModuleMap::createModule
5764+
: &ModuleMap::findOrCreateModule;
5765+
57595766
bool First = true;
57605767
Module *CurrentModule = nullptr;
57615768
RecordData Record;
@@ -5837,11 +5844,8 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
58375844
if (Parent)
58385845
ParentModule = getSubmodule(Parent);
58395846

5840-
// Retrieve this (sub)module from the module map, creating it if
5841-
// necessary.
5842-
CurrentModule =
5843-
ModMap.findOrCreateModule(Name, ParentModule, IsFramework, IsExplicit)
5844-
.first;
5847+
CurrentModule = std::invoke(CreateModule, &ModMap, Name, ParentModule,
5848+
IsFramework, IsExplicit);
58455849

58465850
SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS;
58475851
if (GlobalIndex >= SubmodulesLoaded.size() ||

0 commit comments

Comments
 (0)