[clang] [Serialization] [ASTWriter] Try to not record namespace as much as possible (PR #179178)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 1 23:33:46 PST 2026
https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/179178
See
https://clang.llvm.org/docs/StandardCPlusPlusModules.html#experimental-non-cascading-changes for the full background.
In short, we want to cut off the dependencies to improve the recompilation time.
And namespace is special, as the same namespace can occur in many TUs.
This patch tries to clean up unneeded reference to namespace from other module file. The touched parts are:
- When write on disk lookup table, don't write the merged table. This makes the ownership/dependency much cleaner. For performance, I think the intention was to make the cost of mergin table amortized. But in our internal workloads, I didn't observe any regression if I turn off writing the merged table.
- When writing redeclarations, only write the ID of the first namespace and avoid referenece to all other previous namespaces. (I don't want to write the first namespace either... but it is more complex).
- When writing the name lookup table, try to write the local namespace.
@jtstogel @mpark I want to invite you to test this with your internal workloads to figure out the correctness and the performance impact on this.
I know I can make the change clean for you by inserting code guarded by "if writing named module" but I think it will be much better if we can make the underlying implementation more homogeneous if possible.
>From cc10944f955f7ebe122d5f4e9e3a55487b8e4593 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Mon, 2 Feb 2026 15:16:43 +0800
Subject: [PATCH] [Serialization] [ASTWriter] Try to not record namespace as
much as possible
See
https://clang.llvm.org/docs/StandardCPlusPlusModules.html#experimental-non-cascading-changes
for the full background.
In short, we want to cut off the dependencies to improve the
recompilation time.
And namespace is special, as the same namespace can occur in many TUs.
This patch tries to clean up unneeded reference to namespace from other
module file. The touched parts are:
- When write on disk lookup table, don't write the merged table. This
makes the ownership/dependency much cleaner. For performance, I think
the intention was to make the cost of mergin table amortized. But in
our internal workloads, I didn't observe any regression if I turn off
writing the merged table.
- When writing redeclarations, only write the ID of the first namespace
and avoid referenece to all other previous namespaces. (I don't want to
write the first namespace either... but it is more complex).
- When writing the name lookup table, try to write the local namespace.
---
clang/include/clang/Serialization/ASTWriter.h | 2 -
clang/lib/Serialization/ASTWriter.cpp | 38 ++++++++--------
clang/lib/Serialization/ASTWriterDecl.cpp | 17 +++++--
.../no-transitive-change-namespace.cppm | 44 +++++++++++++++++++
.../Modules/no-transitive-decl-change-4.cppm | 2 +-
5 files changed, 78 insertions(+), 25 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-change-namespace.cppm
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 1c4025f7482d9..867d0c96abca7 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -928,8 +928,6 @@ class ASTWriter : public ASTDeserializationListener,
void handleVTable(CXXRecordDecl *RD);
- void addTouchedModuleFile(serialization::ModuleFile *);
-
private:
// ASTDeserializationListener implementation
void ReaderInitialized(ASTReader *Reader) override;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 3e10bbfedfe65..7e65c3940af5f 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4084,10 +4084,6 @@ void ASTWriter::handleVTable(CXXRecordDecl *RD) {
PendingEmittingVTables.push_back(RD);
}
-void ASTWriter::addTouchedModuleFile(serialization::ModuleFile *MF) {
- TouchedModuleFiles.insert(MF);
-}
-
//===----------------------------------------------------------------------===//
// DeclContext's Name Lookup Table Serialization
//===----------------------------------------------------------------------===//
@@ -4132,7 +4128,6 @@ class ASTDeclContextNameLookupTraitBase {
"have reference to loaded module file but no chain?");
using namespace llvm::support;
- Writer.addTouchedModuleFile(F);
endian::write<uint32_t>(Out, Writer.getChain()->getModuleFileID(F),
llvm::endianness::little);
}
@@ -4330,6 +4325,14 @@ static bool isTULocalInNamedModules(NamedDecl *D) {
return D->getLinkageInternal() == Linkage::Internal;
}
+static NamedDecl *getLocalRedecl(NamedDecl *D) {
+ for (auto *RD : D->redecls())
+ if (!RD->isFromASTFile())
+ return cast<NamedDecl>(RD);
+
+ return D;
+}
+
class ASTDeclContextNameLookupTrait
: public ASTDeclContextNameTrivialLookupTrait {
public:
@@ -4402,6 +4405,13 @@ class ASTDeclContextNameLookupTrait
auto AddDecl = [this](NamedDecl *D) {
NamedDecl *DeclForLocalLookup =
getDeclForLocalLookup(Writer.getLangOpts(), D);
+
+ // Namespace may have many redeclarations in many TU.
+ // Also, these namespaces are symmetric. So that we try to use
+ // the local redecl if any.
+ if (isa<NamespaceDecl>(DeclForLocalLookup) &&
+ DeclForLocalLookup->isFromASTFile())
+ DeclForLocalLookup = getLocalRedecl(DeclForLocalLookup);
if (Writer.getDoneWritingDeclsAndTypes() &&
!Writer.wasDeclEmitted(DeclForLocalLookup))
@@ -4524,7 +4534,6 @@ class LazySpecializationInfoLookupTrait {
"have reference to loaded module file but no chain?");
using namespace llvm::support;
- Writer.addTouchedModuleFile(F);
endian::write<uint32_t>(Out, Writer.getChain()->getModuleFileID(F),
llvm::endianness::little);
}
@@ -4623,7 +4632,7 @@ void ASTWriter::GenerateSpecializationInfoLookupTable(
Generator.insert(HashValue, Trait.getData(Specs, ExisitingSpecs), Trait);
}
- Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr);
+ Generator.emit(LookupTable, Trait, nullptr);
}
uint64_t ASTWriter::WriteSpecializationInfoLookupTable(
@@ -4817,10 +4826,8 @@ void ASTWriter::GenerateNameLookupTable(
Generator.insert(ConversionDecls.front()->getDeclName(),
Trait.getData(ConversionDecls), Trait);
- // Create the on-disk hash table. Also emit the existing imported and
- // merged table if there is one.
- auto *Lookups = Chain ? Chain->getLoadedLookupTables(DC) : nullptr;
- Generator.emit(LookupTable, Trait, Lookups ? &Lookups->Table : nullptr);
+ // Create the on-disk hash table.
+ Generator.emit(LookupTable, Trait, nullptr);
const auto &ModuleLocalDecls = Trait.getModuleLocalDecls();
if (!ModuleLocalDecls.empty()) {
@@ -4836,11 +4843,8 @@ void ASTWriter::GenerateNameLookupTable(
ModuleLocalTrait);
}
- auto *ModuleLocalLookups =
- Chain ? Chain->getModuleLocalLookupTables(DC) : nullptr;
ModuleLocalLookupGenerator.emit(
- ModuleLocalLookupTable, ModuleLocalTrait,
- ModuleLocalLookups ? &ModuleLocalLookups->Table : nullptr);
+ ModuleLocalLookupTable, ModuleLocalTrait, nullptr);
}
const auto &TULocalDecls = Trait.getTULocalDecls();
@@ -4856,9 +4860,7 @@ void ASTWriter::GenerateNameLookupTable(
TULookupGenerator.insert(Key, TULocalTrait.getData(IDs), TULocalTrait);
}
- auto *TULocalLookups = Chain ? Chain->getTULocalLookupTables(DC) : nullptr;
- TULookupGenerator.emit(TULookupTable, TULocalTrait,
- TULocalLookups ? &TULocalLookups->Table : nullptr);
+ TULookupGenerator.emit(TULookupTable, TULocalTrait, nullptr);
}
}
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7646d5d5efe00..e39832f5c8e5c 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2234,8 +2234,15 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
// redecl chain.
unsigned I = Record.size();
Record.push_back(0);
- if (Writer.Chain)
- AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false);
+ if (Writer.Chain) {
+ // Namespace can have many redeclaration in many TU.
+ // We try to avoid touching every redeclaration to try to
+ // reduce the dependencies as much as possible.
+ if (!isa<NamespaceDecl>(DAsT))
+ AddFirstDeclFromEachModule(DAsT, /*IncludeLocal*/false);
+ else
+ Record.AddDeclRef(nullptr);
+ }
// This is the number of imported first declarations + 1.
Record[I] = Record.size() - I;
@@ -2265,8 +2272,10 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
//
// FIXME: This is not correct; when we reach an imported declaration we
// won't emit its previous declaration.
- (void)Writer.GetDeclRef(D->getPreviousDecl());
- (void)Writer.GetDeclRef(MostRecent);
+ if (D->getPreviousDecl() && !D->getPreviousDecl()->isFromASTFile())
+ (void)Writer.GetDeclRef(D->getPreviousDecl());
+ if (!MostRecent->isFromASTFile())
+ (void)Writer.GetDeclRef(MostRecent);
} else {
// We use the sentinel value 0 to indicate an only declaration.
Record.push_back(0);
diff --git a/clang/test/Modules/no-transitive-change-namespace.cppm b/clang/test/Modules/no-transitive-change-namespace.cppm
new file mode 100644
index 0000000000000..f67dd8533a6df
--- /dev/null
+++ b/clang/test/Modules/no-transitive-change-namespace.cppm
@@ -0,0 +1,44 @@
+// Test that adding a new decl in a module which originally only contained a namespace
+// won't break the dependency.
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/Root.cppm -o %t/Root.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/N.cppm -o %t/N.pcm \
+// RUN: -fmodule-file=Root=%t/Root.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/N.v1.cppm -o %t/N.v1.pcm \
+// RUN: -fmodule-file=Root=%t/Root.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/M.cppm -o %t/M.pcm \
+// RUN: -fmodule-file=N=%t/N.pcm -fmodule-file=Root=%t/Root.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/M.cppm -o %t/M.v1.pcm \
+// RUN: -fmodule-file=N=%t/N.v1.pcm -fmodule-file=Root=%t/Root.pcm
+//
+// RUN: diff %t/M.pcm %t/M.v1.pcm &> /dev/null
+
+//--- Root.cppm
+export module Root;
+export namespace N {
+
+}
+
+//--- N.cppm
+export module N;
+import Root;
+export namespace N {
+
+}
+
+//--- N.v1.cppm
+export module N;
+import Root;
+export namespace N {
+struct NN {};
+}
+
+//--- M.cppm
+export module M;
+import N;
+export namespace N {
+struct MM {};
+}
diff --git a/clang/test/Modules/no-transitive-decl-change-4.cppm b/clang/test/Modules/no-transitive-decl-change-4.cppm
index 944878be8df63..bb7e47f32669e 100644
--- a/clang/test/Modules/no-transitive-decl-change-4.cppm
+++ b/clang/test/Modules/no-transitive-decl-change-4.cppm
@@ -27,7 +27,7 @@
// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface %t/B.cppm -o %t/B.v1.pcm \
// RUN: -fprebuilt-module-path=%t -fmodule-file=AWrapper=%t/AWrapper.v1.pcm -fmodule-file=A=%t/A.v1.pcm
//
-// RUN: not diff %t/B.pcm %t/B.v1.pcm &> /dev/null
+// RUN: diff %t/B.pcm %t/B.v1.pcm &> /dev/null
//--- T.cppm
export module T;
More information about the cfe-commits
mailing list