[clang] 0f05096 - [Serialization] Do less redundant work computing affecting module maps (#66933)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 22 02:23:15 PDT 2023
Author: Sam McCall
Date: 2023-09-22T11:23:11+02:00
New Revision: 0f05096540bc90125df47983d7013dd440617055
URL: https://github.com/llvm/llvm-project/commit/0f05096540bc90125df47983d7013dd440617055
DIFF: https://github.com/llvm/llvm-project/commit/0f05096540bc90125df47983d7013dd440617055.diff
LOG: [Serialization] Do less redundant work computing affecting module maps (#66933)
We're traversing the same chains of module ancestors and include
locations repeatedly, despite already populating sets that can detect
it!
This is a problem because translateFile() is expensive. I think we can
avoid it entirely, but this seems like an improvement either way.
I removed a callback indirection rather than giving it a more
complicated
signature, and accordingly renamed the lambdas to be more concrete.
Added:
Modified:
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 65bee806d2c5571..216ca94111e156b 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -163,8 +163,6 @@ namespace {
std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP,
Module *RootModule) {
- std::set<const FileEntry *> ModuleMaps{};
- std::set<const Module *> ProcessedModules;
SmallVector<const Module *> ModulesToProcess{RootModule};
const HeaderSearch &HS = PP.getHeaderSearchInfo();
@@ -195,42 +193,45 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP,
const ModuleMap &MM = HS.getModuleMap();
SourceManager &SourceMgr = PP.getSourceManager();
- auto ForIncludeChain = [&](FileEntryRef F,
- llvm::function_ref<void(FileEntryRef)> CB) {
- CB(F);
+ std::set<const FileEntry *> ModuleMaps{};
+ auto CollectIncludingModuleMaps = [&](FileEntryRef F) {
+ if (!ModuleMaps.insert(F).second)
+ return;
FileID FID = SourceMgr.translateFile(F);
SourceLocation Loc = SourceMgr.getIncludeLoc(FID);
// The include location of inferred module maps can point into the header
// file that triggered the inferring. Cut off the walk if that's the case.
while (Loc.isValid() && isModuleMap(SourceMgr.getFileCharacteristic(Loc))) {
FID = SourceMgr.getFileID(Loc);
- CB(*SourceMgr.getFileEntryRefForID(FID));
+ if (!ModuleMaps.insert(*SourceMgr.getFileEntryRefForID(FID)).second)
+ break;
Loc = SourceMgr.getIncludeLoc(FID);
}
};
- auto ProcessModuleOnce = [&](const Module *M) {
- for (const Module *Mod = M; Mod; Mod = Mod->Parent)
- if (ProcessedModules.insert(Mod).second) {
- auto Insert = [&](FileEntryRef F) { ModuleMaps.insert(F); };
- // The containing module map is affecting, because it's being pointed
- // into by Module::DefinitionLoc.
- if (auto ModuleMapFile = MM.getContainingModuleMapFile(Mod))
- ForIncludeChain(*ModuleMapFile, Insert);
- // For inferred modules, the module map that allowed inferring is not in
- // the include chain of the virtual containing module map file. It did
- // affect the compilation, though.
- if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod))
- ForIncludeChain(*ModuleMapFile, Insert);
- }
+ std::set<const Module *> ProcessedModules;
+ auto CollectIncludingMapsFromAncestors = [&](const Module *M) {
+ for (const Module *Mod = M; Mod; Mod = Mod->Parent) {
+ if (!ProcessedModules.insert(Mod).second)
+ break;
+ // The containing module map is affecting, because it's being pointed
+ // into by Module::DefinitionLoc.
+ if (auto ModuleMapFile = MM.getContainingModuleMapFile(Mod))
+ CollectIncludingModuleMaps(*ModuleMapFile);
+ // For inferred modules, the module map that allowed inferring is not in
+ // the include chain of the virtual containing module map file. It did
+ // affect the compilation, though.
+ if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod))
+ CollectIncludingModuleMaps(*ModuleMapFile);
+ }
};
for (const Module *CurrentModule : ModulesToProcess) {
- ProcessModuleOnce(CurrentModule);
+ CollectIncludingMapsFromAncestors(CurrentModule);
for (const Module *ImportedModule : CurrentModule->Imports)
- ProcessModuleOnce(ImportedModule);
+ CollectIncludingMapsFromAncestors(ImportedModule);
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
- ProcessModuleOnce(UndeclaredModule);
+ CollectIncludingMapsFromAncestors(UndeclaredModule);
}
return ModuleMaps;
More information about the cfe-commits
mailing list