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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
