[clang] 8f4ea36 - [clang] Improve laziness of resolving module map headers.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2022-03-01T15:56:23+01:00
New Revision: 8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f

URL: https://github.com/llvm/llvm-project/commit/8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f
DIFF: https://github.com/llvm/llvm-project/commit/8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f.diff

LOG: [clang] Improve laziness of resolving module map headers.

clang has support for lazy headers in module maps - if size and/or
modtime and provided in the cppmap file, headers are only resolved when
an include directive for a file with that size/modtime is encoutered.

Before this change, the lazy resolution was all-or-nothing per module.
That means as soon as even one file in that module potentially matched
an include, all lazy files in that module were resolved. With this
change, only files with matching size/modtime will be resolved.

The goal is to avoid unnecessary stat() calls on non-included files,
which is especially valuable on networked file systems, with higher
latency.

Differential Revision: https://reviews.llvm.org/D120569

Added: 
    

Modified: 
    clang/include/clang/Lex/ModuleMap.h
    clang/lib/Frontend/FrontendAction.cpp
    clang/lib/Lex/ModuleMap.cpp
    clang/lib/Serialization/ASTWriter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index 26169ae9cee95..538dcc7c4bec1 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -456,8 +456,11 @@ class ModuleMap {
   /// is effectively internal, but is exposed so HeaderSearch can call it.
   void resolveHeaderDirectives(const FileEntry *File) const;
 
-  /// Resolve all lazy header directives for the specified module.
-  void resolveHeaderDirectives(Module *Mod) const;
+  /// Resolve lazy header directives for the specified module. If File is
+  /// provided, only headers with same size and modtime are resolved. If File
+  /// is not set, all headers are resolved.
+  void resolveHeaderDirectives(Module *Mod,
+                               llvm::Optional<const FileEntry *> File) const;
 
   /// Reports errors if a module must not include a specific file.
   ///

diff  --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index c5b9e80356db4..b8f92381613a3 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -331,7 +331,7 @@ static std::error_code collectModuleHeaderIncludes(
     return std::error_code();
 
   // Resolve all lazy header directives to header files.
-  ModMap.resolveHeaderDirectives(Module);
+  ModMap.resolveHeaderDirectives(Module, /*File=*/llvm::None);
 
   // If any headers are missing, we can't build this module. In most cases,
   // diagnostics for this should have already been produced; we only get here

diff  --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 0b136aeb580fe..824b2bb192909 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -482,7 +482,7 @@ void ModuleMap::diagnoseHeaderInclusion(Module *RequestingModule,
 
   if (RequestingModule) {
     resolveUses(RequestingModule, /*Complain=*/false);
-    resolveHeaderDirectives(RequestingModule);
+    resolveHeaderDirectives(RequestingModule, /*File=*/llvm::None);
   }
 
   bool Excluded = false;
@@ -1191,25 +1191,35 @@ void ModuleMap::resolveHeaderDirectives(const FileEntry *File) const {
   auto BySize = LazyHeadersBySize.find(File->getSize());
   if (BySize != LazyHeadersBySize.end()) {
     for (auto *M : BySize->second)
-      resolveHeaderDirectives(M);
+      resolveHeaderDirectives(M, File);
     LazyHeadersBySize.erase(BySize);
   }
 
   auto ByModTime = LazyHeadersByModTime.find(File->getModificationTime());
   if (ByModTime != LazyHeadersByModTime.end()) {
     for (auto *M : ByModTime->second)
-      resolveHeaderDirectives(M);
+      resolveHeaderDirectives(M, File);
     LazyHeadersByModTime.erase(ByModTime);
   }
 }
 
-void ModuleMap::resolveHeaderDirectives(Module *Mod) const {
+void ModuleMap::resolveHeaderDirectives(
+    Module *Mod, llvm::Optional<const FileEntry *> File) const {
   bool NeedsFramework = false;
-  for (auto &Header : Mod->UnresolvedHeaders)
-    // This operation is logically const; we're just changing how we represent
-    // the header information for this file.
-    const_cast<ModuleMap*>(this)->resolveHeader(Mod, Header, NeedsFramework);
-  Mod->UnresolvedHeaders.clear();
+  SmallVector<Module::UnresolvedHeaderDirective, 1> NewHeaders;
+  const auto Size = File ? File.getValue()->getSize() : 0;
+  const auto ModTime = File ? File.getValue()->getModificationTime() : 0;
+
+  for (auto &Header : Mod->UnresolvedHeaders) {
+    if (File && ((Header.ModTime && Header.ModTime != ModTime) ||
+                 (Header.Size && Header.Size != Size)))
+      NewHeaders.push_back(Header);
+    else
+      // This operation is logically const; we're just changing how we represent
+      // the header information for this file.
+      const_cast<ModuleMap *>(this)->resolveHeader(Mod, Header, NeedsFramework);
+  }
+  Mod->UnresolvedHeaders.swap(NewHeaders);
 }
 
 void ModuleMap::addHeader(Module *Mod, Module::Header Header,

diff  --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index cf42e529a8d6a..f0ecfe413c31a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1879,7 +1879,7 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
       // headers list when emitting resolved headers in the first loop below.
       // FIXME: It'd be preferable to avoid doing this if we were given
       // sufficient stat information in the module map.
-      HS.getModuleMap().resolveHeaderDirectives(M);
+      HS.getModuleMap().resolveHeaderDirectives(M, /*File=*/llvm::None);
 
       // If the file didn't exist, we can still create a module if we were given
       // enough information in the module map.