[clang] 2232881 - [C++20] [Modules] Avoid comparing primary module name to decide isInSameModule all the time

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 24 01:55:39 PDT 2024


Author: Chuanqi Xu
Date: 2024-06-24T16:55:17+08:00
New Revision: 2232881736f1a7e3e94ee1123dea1b6cd85a9c3a

URL: https://github.com/llvm/llvm-project/commit/2232881736f1a7e3e94ee1123dea1b6cd85a9c3a
DIFF: https://github.com/llvm/llvm-project/commit/2232881736f1a7e3e94ee1123dea1b6cd85a9c3a.diff

LOG: [C++20] [Modules] Avoid comparing primary module name to decide isInSameModule all the time

Previously, we decide if two module units are in the same module by
comparing name of the primary module interface. But it looks not
efficiency if we always compare the strings. It should be good to
avoid the expensive string operations if possible.

In this patch, we introduced a `llvm::StringMap` to map primary module
name to a Module* and a `llvm::DenseMap<Module*, Module*>` to map a
Module* to a representative Module *. The representative Module* is one
of the Module units belonging to a certain module. The module units have the
same representative Module* should belong to the same module.

We choose the representative Module* by the first module lookup for a
certain primary module name. So the following module units have the same
primary module name would get the same representative modules. So that
for every modules, there will be only one hash process for the primary
module name.

Added: 
    

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/lib/AST/ASTContext.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 46fe2d23e9334..7aa1357b9aad0 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -467,6 +467,14 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// This is the top-level (C++20) Named module we are building.
   Module *CurrentCXXNamedModule = nullptr;
 
+  /// Help structures to decide whether two `const Module *` belongs
+  /// to the same conceptual module to avoid the expensive to string comparison
+  /// if possible.
+  ///
+  /// Not serialized intentionally.
+  llvm::StringMap<const Module *> PrimaryModuleNameMap;
+  llvm::DenseMap<const Module *, const Module *> SameModuleLookupSet;
+
   static constexpr unsigned ConstantArrayTypesLog2InitSize = 8;
   static constexpr unsigned GeneralTypesLog2InitSize = 9;
   static constexpr unsigned FunctionProtoTypesLog2InitSize = 12;

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6aed55a92364b..be24c161d6714 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1114,9 +1114,25 @@ bool ASTContext::isInSameModule(const Module *M1, const Module *M2) {
   if (!M1 != !M2)
     return false;
 
+  /// Get the representative module for M. The representative module is the
+  /// first module unit for a specific primary module name. So that the module
+  /// units have the same representative module belongs to the same module.
+  ///
+  /// The process is helpful to reduce the expensive string operations.
+  auto GetRepresentativeModule = [this](const Module *M) {
+    auto Iter = SameModuleLookupSet.find(M);
+    if (Iter != SameModuleLookupSet.end())
+      return Iter->second;
+
+    const Module *RepresentativeModule =
+        PrimaryModuleNameMap.try_emplace(M->getPrimaryModuleInterfaceName(), M)
+            .first->second;
+    SameModuleLookupSet[M] = RepresentativeModule;
+    return RepresentativeModule;
+  };
+
   assert(M1 && "Shouldn't call `isInSameModule` if both M1 and M2 are none.");
-  return M1->getPrimaryModuleInterfaceName() ==
-         M2->getPrimaryModuleInterfaceName();
+  return GetRepresentativeModule(M1) == GetRepresentativeModule(M2);
 }
 
 ExternCContextDecl *ASTContext::getExternCContextDecl() const {


        


More information about the cfe-commits mailing list