https://github.com/mpark updated https://github.com/llvm/llvm-project/pull/171769
>From b3dc6a5d5d6fd000f76a8dd005d83d2c1b9222dc Mon Sep 17 00:00:00 2001 From: Michael Park <[email protected]> Date: Wed, 10 Dec 2025 17:58:04 -0800 Subject: [PATCH 1/2] [C++20][Modules] Improve namespace handling performance for modules. --- clang/lib/Serialization/ASTReader.cpp | 58 ++++++++++++++++++++++----- clang/lib/Serialization/ASTWriter.cpp | 31 +++++++++++--- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index aec61322fb8be..e8600d1e914cd 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -555,7 +555,25 @@ namespace { using MacroDefinitionsMap = llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>; -using DeclsMap = llvm::DenseMap<DeclarationName, SmallVector<NamedDecl *, 8>>; + +class DeclsSet { + SmallVector<NamedDecl *, 64> Decls; + llvm::SmallPtrSet<NamedDecl *, 8> Found; + +public: + operator ArrayRef<NamedDecl *>() const { return Decls; } + + bool empty() const { return Decls.empty(); } + + bool insert(NamedDecl *ND) { + auto [_, Inserted] = Found.insert(ND); + if (Inserted) + Decls.push_back(ND); + return Inserted; + } +}; + +using DeclsMap = llvm::DenseMap<DeclarationName, DeclsSet>; } // namespace @@ -8702,14 +8720,23 @@ bool ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, return false; // Load the list of declarations. - SmallVector<NamedDecl *, 64> Decls; - llvm::SmallPtrSet<NamedDecl *, 8> Found; + DeclsSet DS; auto Find = [&, this](auto &&Table, auto &&Key) { for (GlobalDeclID ID : Table.find(Key)) { NamedDecl *ND = cast<NamedDecl>(GetDecl(ID)); - if (ND->getDeclName() == Name && Found.insert(ND).second) - Decls.push_back(ND); + if (ND->getDeclName() != Name) + continue; + // Special case for namespaces: There can be a lot of redeclarations of + // some namespaces, and we import a "key declaration" per imported module. + // Since all declarations of a namespace are essentially interchangeable, + // we can optimize namespace look-up by only storing the key declaration + // of the current TU, rather than storing N key declarations where N is + // the # of imported modules that declare that namespace. + // TODO: Try to generalize this optimization to other redeclarable decls. + if (isa<NamespaceDecl>(ND)) + ND = cast<NamedDecl>(getKeyDeclaration(ND)); + DS.insert(ND); } }; @@ -8744,8 +8771,8 @@ bool ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, Find(It->second.Table, Name); } - SetExternalVisibleDeclsForName(DC, Name, Decls); - return !Decls.empty(); + SetExternalVisibleDeclsForName(DC, Name, DS); + return !DS.empty(); } void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) { @@ -8763,7 +8790,16 @@ void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) { for (GlobalDeclID ID : It->second.Table.findAll()) { NamedDecl *ND = cast<NamedDecl>(GetDecl(ID)); - Decls[ND->getDeclName()].push_back(ND); + // Special case for namespaces: There can be a lot of redeclarations of + // some namespaces, and we import a "key declaration" per imported module. + // Since all declarations of a namespace are essentially interchangeable, + // we can optimize namespace look-up by only storing the key declaration + // of the current TU, rather than storing N key declarations where N is + // the # of imported modules that declare that namespace. + // TODO: Try to generalize this optimization to other redeclarable decls. + if (isa<NamespaceDecl>(ND)) + ND = cast<NamedDecl>(getKeyDeclaration(ND)); + Decls[ND->getDeclName()].insert(ND); } // FIXME: Why a PCH test is failing if we remove the iterator after findAll? @@ -8773,9 +8809,9 @@ void ASTReader::completeVisibleDeclsMap(const DeclContext *DC) { findAll(ModuleLocalLookups, NumModuleLocalVisibleDeclContexts); findAll(TULocalLookups, NumTULocalVisibleDeclContexts); - for (DeclsMap::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I) { - SetExternalVisibleDeclsForName(DC, I->first, I->second); - } + for (auto &[Name, DS] : Decls) + SetExternalVisibleDeclsForName(DC, Name, DS); + const_cast<DeclContext *>(DC)->setHasExternalVisibleStorage(false); } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index dc7f93e0483e4..6a4b788066476 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4396,20 +4396,20 @@ class ASTDeclContextNameLookupTrait template <typename Coll> data_type getData(const Coll &Decls) { unsigned Start = DeclIDs.size(); - for (NamedDecl *D : Decls) { + auto AddDecl = [this](NamedDecl *D) { NamedDecl *DeclForLocalLookup = getDeclForLocalLookup(Writer.getLangOpts(), D); if (Writer.getDoneWritingDeclsAndTypes() && !Writer.wasDeclEmitted(DeclForLocalLookup)) - continue; + return; // Try to avoid writing internal decls to reduced BMI. // See comments in ASTWriter::WriteDeclContextLexicalBlock for details. if (Writer.isGeneratingReducedBMI() && !DeclForLocalLookup->isFromExplicitGlobalModule() && IsInternalDeclFromFileContext(DeclForLocalLookup)) - continue; + return; auto ID = Writer.GetDeclRef(DeclForLocalLookup); @@ -4423,7 +4423,7 @@ class ASTDeclContextNameLookupTrait ModuleLocalDeclsMap.insert({Key, DeclIDsTy{ID}}); else Iter->second.push_back(ID); - continue; + return; } break; case LookupVisibility::TULocal: { @@ -4432,7 +4432,7 @@ class ASTDeclContextNameLookupTrait TULocalDeclsMap.insert({D->getDeclName(), DeclIDsTy{ID}}); else Iter->second.push_back(ID); - continue; + return; } case LookupVisibility::GenerallyVisibile: // Generally visible decls go into the general lookup table. @@ -4440,6 +4440,27 @@ class ASTDeclContextNameLookupTrait } DeclIDs.push_back(ID); + }; + for (NamedDecl *D : Decls) { + if (isa<NamespaceDecl>(D) && D->isFromASTFile()) { + // In ASTReader, we stored only the key declaration of a namespace decl + // for this TU rather than storing all of the key declarations from each + // imported module. If we have an external namespace decl, this is that + // key declaration and we need to re-expand it to write out all of the + // key declarations from each imported module again. + // + // See comment 'ASTReader::FindExternalVisibleDeclsByName' for details. + ASTReader *Chain = Writer.getChain(); + assert(Chain && "An external namespace decl without an ASTReader"); + assert(D == Chain->getKeyDeclaration(D) && + "An external namespace decl that is not " + "the key declaration of this TU"); + Chain->forEachImportedKeyDecl(D, [&AddDecl](const Decl *D) { + AddDecl(cast<NamedDecl>(const_cast<Decl *>(D))); + }); + } else { + AddDecl(D); + } } return std::make_pair(Start, DeclIDs.size()); } >From 3b2ecdc27ad402fc23c9c62663b281a0a3e4fdd0 Mon Sep 17 00:00:00 2001 From: Michael Park <[email protected]> Date: Mon, 15 Dec 2025 08:10:02 -0800 Subject: [PATCH 2/2] [C++20][Modules] Refine condition for key decl check. --- clang/lib/Serialization/ASTWriter.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 6a4b788066476..899fd69c2045e 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4442,7 +4442,9 @@ class ASTDeclContextNameLookupTrait DeclIDs.push_back(ID); }; for (NamedDecl *D : Decls) { - if (isa<NamespaceDecl>(D) && D->isFromASTFile()) { + if (ASTReader *Chain = Writer.getChain(); + Chain && isa<NamespaceDecl>(D) && D->isFromASTFile() && + D == Chain->getKeyDeclaration(D)) { // In ASTReader, we stored only the key declaration of a namespace decl // for this TU rather than storing all of the key declarations from each // imported module. If we have an external namespace decl, this is that @@ -4450,11 +4452,6 @@ class ASTDeclContextNameLookupTrait // key declarations from each imported module again. // // See comment 'ASTReader::FindExternalVisibleDeclsByName' for details. - ASTReader *Chain = Writer.getChain(); - assert(Chain && "An external namespace decl without an ASTReader"); - assert(D == Chain->getKeyDeclaration(D) && - "An external namespace decl that is not " - "the key declaration of this TU"); Chain->forEachImportedKeyDecl(D, [&AddDecl](const Decl *D) { AddDecl(cast<NamedDecl>(const_cast<Decl *>(D))); }); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
