[clang][deps] Optimize in-process timestamping of PCMs

jansvoboda11 · jansvoboda11 · commit f5b5bc95209c · 2025-05-01T07:38:47.000-07:00
diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h
@@ -12,6 +12,8 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 
+#include <ctime>
+
 namespace llvm {
 class AdvisoryLock;
 } // namespace llvm
@@ -31,11 +33,19 @@ class ModuleCache : public RefCountedBase<ModuleCache> {
   virtual std::unique_ptr<llvm::AdvisoryLock>
   getLock(StringRef ModuleFilename) = 0;
 
+  /// Returns the timestamp denoting the last time inputs of the module file
+  /// were validated.
+  virtual std::time_t getModuleTimestamp(StringRef ModuleFilename) = 0;
+
+  /// Updates the timestamp denoting the last time inputs of the module file
+  /// were validated.
+  virtual void updateModuleTimestamp(StringRef ModuleFilename) = 0;
+
   /// Returns this process's view of the module cache.
   virtual InMemoryModuleCache &getInMemoryModuleCache() = 0;
   virtual const InMemoryModuleCache &getInMemoryModuleCache() const = 0;
 
-  // TODO: Virtualize writing/reading PCM files, timestamping, pruning, etc.
+  // TODO: Virtualize writing/reading PCM files, pruning, etc.
 
   virtual ~ModuleCache() = default;
 };
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -100,7 +100,7 @@ class DependencyScanningService {
     return SharedCache;
   }
 
-  ModuleCacheMutexes &getModuleCacheMutexes() { return ModCacheMutexes; }
+  ModuleCacheEntries &getModuleCacheEntries() { return ModCacheEntries; }
 
 private:
   const ScanningMode Mode;
@@ -113,8 +113,8 @@ class DependencyScanningService {
   const bool TraceVFS;
   /// The global file system cache.
   DependencyScanningFilesystemSharedCache SharedCache;
-  /// The global module cache mutexes.
-  ModuleCacheMutexes ModCacheMutexes;
+  /// The global module cache entries.
+  ModuleCacheEntries ModCacheEntries;
 };
 
 } // end namespace dependencies
diff --git a/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h
@@ -18,13 +18,18 @@
 namespace clang {
 namespace tooling {
 namespace dependencies {
-struct ModuleCacheMutexes {
+struct ModuleCacheEntry {
+  std::shared_mutex CompilationMutex;
+  std::atomic<std::time_t> Timestamp = 0;
+};
+
+struct ModuleCacheEntries {
   std::mutex Mutex;
-  llvm::StringMap<std::unique_ptr<std::shared_mutex>> Map;
+  llvm::StringMap<std::unique_ptr<ModuleCacheEntry>> Map;
 };
 
 IntrusiveRefCntPtr<ModuleCache>
-makeInProcessModuleCache(ModuleCacheMutexes &Mutexes);
+makeInProcessModuleCache(ModuleCacheEntries &Entries);
 } // namespace dependencies
 } // namespace tooling
 } // namespace clang
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
@@ -510,15 +510,3 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
     return false;
   return isa<TagDecl, FieldDecl>(D);
 }
-
-void serialization::updateModuleTimestamp(StringRef ModuleFilename) {
-  // Overwrite the timestamp file contents so that file's mtime changes.
-  std::error_code EC;
-  llvm::raw_fd_ostream OS(ModuleFile::getTimestampFilename(ModuleFilename), EC,
-                          llvm::sys::fs::OF_TextWithCRLF);
-  if (EC)
-    return;
-  OS << "Timestamp file\n";
-  OS.close();
-  OS.clear_error(); // Avoid triggering a fatal error.
-}
diff --git a/clang/lib/Serialization/ASTCommon.h b/clang/lib/Serialization/ASTCommon.h
@@ -100,8 +100,6 @@ inline bool isPartOfPerModuleInitializer(const Decl *D) {
   return false;
 }
 
-void updateModuleTimestamp(StringRef ModuleFilename);
-
 } // namespace serialization
 
 } // namespace clang
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
@@ -3102,6 +3102,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
         // files.
 
         unsigned N = ValidateSystemInputs ? NumInputs : NumUserInputs;
+        // FIXME: F.InputFilesValidationTimestamp comes from std::chrono, but
+        // HSOpts.BuildSessionTimestamp comes from the FS. They may not be
+        // comparable.
         if (HSOpts.ModulesValidateOncePerBuildSession &&
             F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp &&
             F.Kind == MK_ImplicitModule)
@@ -4950,9 +4953,13 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type,
     // timestamp files are up-to-date in this build session.
     for (unsigned I = 0, N = Loaded.size(); I != N; ++I) {
       ImportedModule &M = Loaded[I];
+      // FIXME: F.InputFilesValidationTimestamp comes from std::chrono, but
+      // HSOpts.BuildSessionTimestamp comes from the FS. They may not be
+      // comparable.
       if (M.Mod->Kind == MK_ImplicitModule &&
           M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
-        updateModuleTimestamp(M.Mod->FileName);
+        getModuleManager().getModuleCache().updateModuleTimestamp(
+            M.Mod->FileName);
     }
   }
 
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
@@ -5394,7 +5394,7 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject,
   if (WritingModule && PPRef.getHeaderSearchInfo()
                            .getHeaderSearchOpts()
                            .ModulesValidateOncePerBuildSession)
-    updateModuleTimestamp(OutputFile);
+    ModCache.updateModuleTimestamp(OutputFile);
 
   if (ShouldCacheASTInMemory) {
     // Construct MemoryBuffer and update buffer manager.
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
@@ -9,6 +9,7 @@
 #include "clang/Serialization/ModuleCache.h"
 
 #include "clang/Serialization/InMemoryModuleCache.h"
+#include "clang/Serialization/ModuleFile.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/LockFileManager.h"
 #include "llvm/Support/Path.h"
@@ -32,6 +33,28 @@ class CrossProcessModuleCache : public ModuleCache {
     return std::make_unique<llvm::LockFileManager>(ModuleFilename);
   }
 
+  std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
+    std::string TimestampFilename =
+        serialization::ModuleFile::getTimestampFilename(ModuleFilename);
+    llvm::sys::fs::file_status Status;
+    if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
+      return {};
+    return llvm::sys::toTimeT(Status.getLastModificationTime());
+  }
+
+  void updateModuleTimestamp(StringRef ModuleFilename) override {
+    // Overwrite the timestamp file contents so that file's mtime changes.
+    std::error_code EC;
+    llvm::raw_fd_ostream OS(
+        serialization::ModuleFile::getTimestampFilename(ModuleFilename), EC,
+        llvm::sys::fs::OF_TextWithCRLF);
+    if (EC)
+      return;
+    OS << "Timestamp file\n";
+    OS.close();
+    OS.clear_error(); // Avoid triggering a fatal error.
+  }
+
   InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; }
   const InMemoryModuleCache &getInMemoryModuleCache() const override {
     return InMemory;
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
@@ -174,15 +174,9 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
   NewModule->ImportLoc = ImportLoc;
   NewModule->InputFilesValidationTimestamp = 0;
 
-  if (NewModule->Kind == MK_ImplicitModule) {
-    std::string TimestampFilename =
-        ModuleFile::getTimestampFilename(NewModule->FileName);
-    llvm::vfs::Status Status;
-    // A cached stat value would be fine as well.
-    if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
-      NewModule->InputFilesValidationTimestamp =
-          llvm::sys::toTimeT(Status.getLastModificationTime());
-  }
+  if (NewModule->Kind == MK_ImplicitModule)
+    NewModule->InputFilesValidationTimestamp =
+        ModCache->getModuleTimestamp(NewModule->FileName);
 
   // Load the contents of the module
   if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -411,7 +411,7 @@ class DependencyScanningAction : public tooling::ToolAction {
     Scanned = true;
 
     // Create a compiler instance to handle the actual work.
-    auto ModCache = makeInProcessModuleCache(Service.getModuleCacheMutexes());
+    auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries());
     ScanInstanceStorage.emplace(std::move(Invocation),
                                 std::move(PCHContainerOps), ModCache.get());
     CompilerInstance &ScanInstance = *ScanInstanceStorage;
diff --git a/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "llvm/Support/AdvisoryLock.h"
+#include "llvm/Support/Chrono.h"
 
 #include <mutex>
 
@@ -50,7 +51,7 @@ class ReaderWriterLock : public llvm::AdvisoryLock {
 };
 
 class InProcessModuleCache : public ModuleCache {
-  ModuleCacheMutexes &Mutexes;
+  ModuleCacheEntries &Entries;
 
   // TODO: If we changed the InMemoryModuleCache API and relied on strict
   // context hash, we could probably create more efficient thread-safe
@@ -59,19 +60,46 @@ class InProcessModuleCache : public ModuleCache {
   InMemoryModuleCache InMemory;
 
 public:
-  InProcessModuleCache(ModuleCacheMutexes &Mutexes) : Mutexes(Mutexes) {}
+  InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {}
 
   void prepareForGetLock(StringRef Filename) override {}
 
   std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override {
-    auto &Mtx = [&]() -> std::shared_mutex & {
-      std::lock_guard<std::mutex> Lock(Mutexes.Mutex);
-      auto &Mutex = Mutexes.Map[Filename];
-      if (!Mutex)
-        Mutex = std::make_unique<std::shared_mutex>();
-      return *Mutex;
+    auto &CompilationMutex = [&]() -> std::shared_mutex & {
+      std::lock_guard Lock(Entries.Mutex);
+      auto &Entry = Entries.Map[Filename];
+      if (!Entry)
+        Entry = std::make_unique<ModuleCacheEntry>();
+      return Entry->CompilationMutex;
     }();
-    return std::make_unique<ReaderWriterLock>(Mtx);
+    return std::make_unique<ReaderWriterLock>(CompilationMutex);
+  }
+
+  std::time_t getModuleTimestamp(StringRef Filename) override {
+    auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
+      std::lock_guard Lock(Entries.Mutex);
+      auto &Entry = Entries.Map[Filename];
+      if (!Entry)
+        Entry = std::make_unique<ModuleCacheEntry>();
+      return Entry->Timestamp;
+    }();
+
+    return Timestamp.load();
+  }
+
+  void updateModuleTimestamp(StringRef Filename) override {
+    // Note: This essentially replaces FS contention with mutex contention.
+    auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
+      std::lock_guard Lock(Entries.Mutex);
+      auto &Entry = Entries.Map[Filename];
+      if (!Entry)
+        Entry = std::make_unique<ModuleCacheEntry>();
+      return Entry->Timestamp;
+    }();
+
+    std::time_t Expected = 0;
+    std::time_t Now = llvm::sys::toTimeT(std::chrono::system_clock::now());
+    Timestamp.compare_exchange_weak(Expected, Now);
   }
 
   InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; }
@@ -82,6 +110,6 @@ class InProcessModuleCache : public ModuleCache {
 } // namespace
 
 IntrusiveRefCntPtr<ModuleCache>
-dependencies::makeInProcessModuleCache(ModuleCacheMutexes &Mutexes) {
-  return llvm::makeIntrusiveRefCnt<InProcessModuleCache>(Mutexes);
+dependencies::makeInProcessModuleCache(ModuleCacheEntries &Entries) {
+  return llvm::makeIntrusiveRefCnt<InProcessModuleCache>(Entries);
 }

Original file line number	Diff line number	Diff line change
`@@ -100,8 +100,6 @@ inline bool isPartOfPerModuleInitializer(const Decl *D) {`
`100`	`100`	`return false;`
`101`	`101`	`}`
`102`	`102`
`103`		`-void updateModuleTimestamp(StringRef ModuleFilename);`
`104`		`-`
`105`	`103`	`} // namespace serialization`
`106`	`104`
`107`	`105`	`} // namespace clang`