Skip to content

Commit 452b22f

Browse files
sam-mccalljansvoboda11
authored andcommitted
[Serialization] Do less redundant work computing affecting module maps (llvm#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. (cherry picked from commit 0f05096)
1 parent 7d0518c commit 452b22f

File tree

1 file changed

+24
-23
lines changed

1 file changed

+24
-23
lines changed

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,6 @@ namespace {
163163

164164
std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP,
165165
Module *RootModule) {
166-
std::set<const FileEntry *> ModuleMaps{};
167-
std::set<const Module *> ProcessedModules;
168166
SmallVector<const Module *> ModulesToProcess{RootModule};
169167

170168
const HeaderSearch &HS = PP.getHeaderSearchInfo();
@@ -195,42 +193,45 @@ std::set<const FileEntry *> GetAffectingModuleMaps(const Preprocessor &PP,
195193
const ModuleMap &MM = HS.getModuleMap();
196194
SourceManager &SourceMgr = PP.getSourceManager();
197195

198-
auto ForIncludeChain = [&](FileEntryRef F,
199-
llvm::function_ref<void(FileEntryRef)> CB) {
200-
CB(F);
196+
std::set<const FileEntry *> ModuleMaps{};
197+
auto CollectIncludingModuleMaps = [&](FileEntryRef F) {
198+
if (!ModuleMaps.insert(F).second)
199+
return;
201200
FileID FID = SourceMgr.translateFile(F);
202201
SourceLocation Loc = SourceMgr.getIncludeLoc(FID);
203202
// The include location of inferred module maps can point into the header
204203
// file that triggered the inferring. Cut off the walk if that's the case.
205204
while (Loc.isValid() && isModuleMap(SourceMgr.getFileCharacteristic(Loc))) {
206205
FID = SourceMgr.getFileID(Loc);
207-
CB(*SourceMgr.getFileEntryRefForID(FID));
206+
if (!ModuleMaps.insert(*SourceMgr.getFileEntryRefForID(FID)).second)
207+
break;
208208
Loc = SourceMgr.getIncludeLoc(FID);
209209
}
210210
};
211211

212-
auto ProcessModuleOnce = [&](const Module *M) {
213-
for (const Module *Mod = M; Mod; Mod = Mod->Parent)
214-
if (ProcessedModules.insert(Mod).second) {
215-
auto Insert = [&](FileEntryRef F) { ModuleMaps.insert(F); };
216-
// The containing module map is affecting, because it's being pointed
217-
// into by Module::DefinitionLoc.
218-
if (auto ModuleMapFile = MM.getContainingModuleMapFile(Mod))
219-
ForIncludeChain(*ModuleMapFile, Insert);
220-
// For inferred modules, the module map that allowed inferring is not in
221-
// the include chain of the virtual containing module map file. It did
222-
// affect the compilation, though.
223-
if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod))
224-
ForIncludeChain(*ModuleMapFile, Insert);
225-
}
212+
std::set<const Module *> ProcessedModules;
213+
auto CollectIncludingMapsFromAncestors = [&](const Module *M) {
214+
for (const Module *Mod = M; Mod; Mod = Mod->Parent) {
215+
if (!ProcessedModules.insert(Mod).second)
216+
break;
217+
// The containing module map is affecting, because it's being pointed
218+
// into by Module::DefinitionLoc.
219+
if (auto ModuleMapFile = MM.getContainingModuleMapFile(Mod))
220+
CollectIncludingModuleMaps(*ModuleMapFile);
221+
// For inferred modules, the module map that allowed inferring is not in
222+
// the include chain of the virtual containing module map file. It did
223+
// affect the compilation, though.
224+
if (auto ModuleMapFile = MM.getModuleMapFileForUniquing(Mod))
225+
CollectIncludingModuleMaps(*ModuleMapFile);
226+
}
226227
};
227228

228229
for (const Module *CurrentModule : ModulesToProcess) {
229-
ProcessModuleOnce(CurrentModule);
230+
CollectIncludingMapsFromAncestors(CurrentModule);
230231
for (const Module *ImportedModule : CurrentModule->Imports)
231-
ProcessModuleOnce(ImportedModule);
232+
CollectIncludingMapsFromAncestors(ImportedModule);
232233
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
233-
ProcessModuleOnce(UndeclaredModule);
234+
CollectIncludingMapsFromAncestors(UndeclaredModule);
234235
}
235236

236237
return ModuleMaps;

0 commit comments

Comments
 (0)