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>;
-using DeclsMap = llvm::DenseMap>;
+
+class DeclsSet {
+ SmallVector Decls;
+ llvm::SmallPtrSet Found;
+
+public:
+ operator ArrayRef() 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;
} // namespace
@@ -8702,14 +8720,23 @@ bool ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
return false;
// Load the list of declarations.
- SmallVector Decls;
- llvm::SmallPtrSet Found;
+ DeclsSet DS;
auto Find = [&, this](auto &&Table, auto &&Key) {
for (GlobalDeclID ID : Table.find(Key)) {
NamedDecl *ND = cast(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(ND))
+ ND = cast(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(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(ND))
+ ND = cast(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(DC)->setHasExternalVisibleStorage(false);
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index dc7f93e0483e4..899fd69c2045e 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4396,20 +4396,20 @@ class ASTDeclContextNameLookupTrait
template 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,24 @@ class ASTDeclContextNameLookupTrait
}
DeclIDs.push_back(ID);
+ };
+ for (NamedDecl *D : Decls) {
+ if (ASTReader *Chain = Writer.getChain();
+ Chain && isa(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
+ // 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.
+ Chain->forEachImportedKeyDecl(D, [&AddDecl](const Decl *D) {
+ AddDecl(cast(const_cast(D)));
+ });
+ } else {
+ AddDecl(D);
+ }
}
return std::make_pair(Start, DeclIDs.size());
}
diff --git a/clang/unittests/Serialization/CMakeLists.txt b/clang/unittests/Serialization/CMakeLists.txt
index 6782e6b4d7330..a5cc1ed83af49 100644
--- a/clang/unittests/Serialization/CMakeLists.txt
+++ b/clang/unittests/Serialization/CMakeLists.txt
@@ -2,6 +2,7 @@ add_clang_unittest(SerializationTests
ForceCheckFileInputTest.cpp
InMemoryModuleCacheTest.cpp
ModuleCacheTest.cpp
+ NamespaceLookupTest.cpp
NoCommentsTest.cpp
PreambleInNamedModulesTest.cpp
LoadSpecLazilyTest.cpp
diff --git a/clang/unittests/Serialization/NamespaceLookupTest.cpp b/clang/unittests/Serialization/NamespaceLookupTest.cpp
new file mode 100644
index 0000000000000..eefa4be9fbee5
--- /dev/null
+++ b/clang/unittests/Serialization/NamespaceLookupTest.cpp
@@ -0,0 +1,247 @@
+//== unittests/Serialization/NamespaceLookupOptimizationTest.cpp =======//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Driver/CreateInvocationFromArgs.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Serialization/ASTReader.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+namespace {
+
+class NamespaceLookupTest : public ::testing::Test {
+ void SetUp() override {
+ ASSERT_FALSE(
+ sys::fs::createUniqueDirectory("namespace-lookup-test", TestDir));
+ }
+
+ void TearDown() override { sys::fs::remove_directories(TestDir); }
+
+public:
+ SmallString<256> TestDir;
+
+ void addFile(StringRef Path, StringRef Contents) {
+ ASSERT_FALSE(sys::path::is_absolute(Path));
+
+ SmallString<256> AbsPath(TestDir);
+ sys::path::append(AbsPath, Path);
+
+ ASSERT_FALSE(
+ sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
+
+ std::error_code EC;
+ llvm::raw_fd_ostream OS(AbsPath, EC);
+ ASSERT_FALSE(EC);
+ OS << Contents;
+ }
+
+ std::string GenerateModuleInterface(StringRef ModuleName,
+ StringRef Contents) {
+ std::string FileName = llvm::Twine(ModuleName + ".cppm").str();
+ addFile(FileName, Contents);
+
+ IntrusiveRefCntPtr VFS =
+ llvm::vfs::createPhysicalFileSystem();
+ DiagnosticOptions DiagOpts;
+ IntrusiveRefCntPtr Diags =
+ CompilerInstance::createDiagnostics(*VFS, DiagOpts);
+ CreateInvocationOptions CIOpts;
+ CIOpts.Diags = Diags;
+ CIOpts.VFS = VFS;
+
+ std::string CacheBMIPath =
+ llvm::Twine(TestDir + "/" + ModuleName + ".pcm").str();
+ std::string PrebuiltModulePath =
+ "-fprebuilt-module-path=" + TestDir.str().str();
+ const char *Args[] = {"clang++",
+ "-std=c++20",
+ "--precompile",
+ PrebuiltModulePath.c_str(),
+ "-working-directory",
+ TestDir.c_str(),
+ "-I",
+ TestDir.c_str(),
+ FileName.c_str(),
+ "-o",
+ CacheBMIPath.c_str()};
+ std::shared_ptr Invocation =
+ createInvocation(Args, CIOpts);
+ EXPECT_TRUE(Invocation);
+
+ CompilerInstance Instance(std::move(Invocation));
+ Instance.setDiagnostics(Diags);
+ Instance.getFrontendOpts().OutputFile = CacheBMIPath;
+ // Avoid memory leaks.
+ Instance.getFrontendOpts().DisableFree = false;
+ GenerateModuleInterfaceAction Action;
+ EXPECT_TRUE(Instance.ExecuteAction(Action));
+ EXPECT_FALSE(Diags->hasErrorOccurred());
+
+ return CacheBMIPath;
+ }
+};
+
+struct NamespaceLookupResult {
+ int NumLocalNamespaces = 0;
+ int NumExternalNamespaces = 0;
+};
+
+class NamespaceLookupConsumer : public ASTConsumer {
+ NamespaceLookupResult &Result;
+
+public:
+ explicit NamespaceLookupConsumer(NamespaceLookupResult &Result)
+ : Result(Result) {}
+
+ void HandleTranslationUnit(ASTContext &Context) override {
+ TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
+ ASSERT_TRUE(TU);
+ ASTReader *Chain = dyn_cast_or_null(Context.getExternalSource());
+ ASSERT_TRUE(Chain);
+ for (const Decl *D :
+ TU->lookup(DeclarationName(&Context.Idents.get("N")))) {
+ if (!isa(D))
+ continue;
+ if (!D->isFromASTFile()) {
+ ++Result.NumLocalNamespaces;
+ } else {
+ ++Result.NumExternalNamespaces;
+ EXPECT_EQ(D, Chain->getKeyDeclaration(D));
+ }
+ }
+ }
+};
+
+class NamespaceLookupAction : public ASTFrontendAction {
+ NamespaceLookupResult &Result;
+
+public:
+ explicit NamespaceLookupAction(NamespaceLookupResult &Result)
+ : Result(Result) {}
+
+ std::unique_ptr
+ CreateASTConsumer(CompilerInstance &CI, StringRef /*Unused*/) override {
+ return std::make_unique(Result);
+ }
+};
+
+TEST_F(NamespaceLookupTest, ExternalNamespacesOnly) {
+ GenerateModuleInterface("M1", R"cpp(
+export module M1;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M2", R"cpp(
+export module M2;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M3", R"cpp(
+export module M3;
+namespace N {}
+ )cpp");
+ const char *test_file_contents = R"cpp(
+import M1;
+import M2;
+import M3;
+ )cpp";
+ std::string DepArg = "-fprebuilt-module-path=" + TestDir.str().str();
+ NamespaceLookupResult Result;
+ EXPECT_TRUE(runToolOnCodeWithArgs(
+ std::make_unique(Result), test_file_contents,
+ {
+ "-std=c++20",
+ DepArg.c_str(),
+ "-I",
+ TestDir.c_str(),
+ },
+ "main.cpp"));
+
+ EXPECT_EQ(0, Result.NumLocalNamespaces);
+ EXPECT_EQ(1, Result.NumExternalNamespaces);
+}
+
+TEST_F(NamespaceLookupTest, ExternalReplacedByLocal) {
+ GenerateModuleInterface("M1", R"cpp(
+export module M1;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M2", R"cpp(
+export module M2;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M3", R"cpp(
+export module M3;
+namespace N {}
+ )cpp");
+ const char *test_file_contents = R"cpp(
+import M1;
+import M2;
+import M3;
+
+namespace N {}
+ )cpp";
+ std::string DepArg = "-fprebuilt-module-path=" + TestDir.str().str();
+ NamespaceLookupResult Result;
+ EXPECT_TRUE(runToolOnCodeWithArgs(
+ std::make_unique(Result), test_file_contents,
+ {
+ "-std=c++20",
+ DepArg.c_str(),
+ "-I",
+ TestDir.c_str(),
+ },
+ "main.cpp"));
+
+ EXPECT_EQ(1, Result.NumLocalNamespaces);
+ EXPECT_EQ(0, Result.NumExternalNamespaces);
+}
+
+TEST_F(NamespaceLookupTest, LocalAndExternalInterleaved) {
+ GenerateModuleInterface("M1", R"cpp(
+export module M1;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M2", R"cpp(
+export module M2;
+namespace N {}
+ )cpp");
+ GenerateModuleInterface("M3", R"cpp(
+export module M3;
+namespace N {}
+ )cpp");
+ const char *test_file_contents = R"cpp(
+import M1;
+
+namespace N {}
+
+import M2;
+import M3;
+ )cpp";
+ std::string DepArg = "-fprebuilt-module-path=" + TestDir.str().str();
+ NamespaceLookupResult Result;
+ EXPECT_TRUE(runToolOnCodeWithArgs(
+ std::make_unique(Result), test_file_contents,
+ {
+ "-std=c++20",
+ DepArg.c_str(),
+ "-I",
+ TestDir.c_str(),
+ },
+ "main.cpp"));
+
+ EXPECT_EQ(1, Result.NumLocalNamespaces);
+ EXPECT_EQ(1, Result.NumExternalNamespaces);
+}
+
+} // namespace