[clang] [C++20][Modules] Improve namespace look-up performance for modules. (PR #171769)
Michael Park via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 10 22:52:01 PST 2025
https://github.com/mpark updated https://github.com/llvm/llvm-project/pull/171769
>From bcff2a7a939c99031315ee9ac345451bbc41d6b9 Mon Sep 17 00:00:00 2001
From: Michael Park <mcypark at gmail.com>
Date: Wed, 10 Dec 2025 17:58:04 -0800
Subject: [PATCH] [C++20][Modules] Improve namespace handling performance for
modules.
---
clang/lib/Serialization/ASTReader.cpp | 57 +++++++++++++++++++++------
clang/lib/Serialization/ASTWriter.cpp | 31 ++++++++++++---
2 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aec61322fb8be..5f66beefd6388 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -555,7 +555,26 @@ 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:
+ NamedDecl *const *data() const { return Decls.data(); }
+
+ bool empty() const { return Decls.empty(); }
+ size_t size() const { return Decls.size(); }
+
+ 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 +8721,22 @@ 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.
+ 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,15 @@ 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.
+ 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 +8808,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 667e04049dac8..971f5991305f7 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 "
+ "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());
}
More information about the cfe-commits
mailing list