[clang] [clang][DependencyScanning] Write the module timestamp once per build session (PR #210587)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Taimuraz Kaitmazov (atassis)

<details>
<summary>Changes</summary>

Under `-fmodules-validate-once-per-build-session`, two dependency-scanning workers that share an implicitly-built module can both find its timestamp stale and record it. The value is the same either way, but per-module work and the `timestamp_write` log should not scale with worker count, and the extra write made `ClangScanDeps/logging-two-threads.c` flaky. Record the timestamp once under the cache entry's lock so only the first worker writes it.

AI assistance: drafted with AI help; the fix and its testing authored and validated by me.

---
Full diff: https://github.com/llvm/llvm-project/pull/210587.diff


2 Files Affected:

- (modified) clang/lib/DependencyScanning/InProcessModuleCache.cpp (+7-2) 
- (modified) clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp (+28) 


``````````diff
diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
index bc6ff88c57323..3608a18dd7794 100644
--- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
@@ -128,10 +128,15 @@ class InProcessModuleCache : public ModuleCache {
 
   void updateModuleTimestamp(StringRef Filename) override {
     // Note: This essentially replaces FS contention with mutex contention.
-    auto &Timestamp = getOrCreateEntry(Filename).Timestamp;
+    ModuleCacheEntry &Entry = getOrCreateEntry(Filename);
+    std::lock_guard<std::mutex> Lock(Entry.Mutex);
 
+    // A shared module may be validated by several workers at once; only the
+    // first records its timestamp (0 means "not recorded yet").
+    if (Entry.Timestamp)
+      return;
+    Entry.Timestamp = llvm::sys::toTimeT(std::chrono::system_clock::now());
     Logger.log() << "timestamp_write: " << Filename;
-    Timestamp.store(llvm::sys::toTimeT(std::chrono::system_clock::now()));
   }
 
   void maybePrune(StringRef Path, time_t PruneInterval,
diff --git a/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp b/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp
index 5b667a4989f50..9c11602f2fe07 100644
--- a/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp
+++ b/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp
@@ -74,3 +74,31 @@ TEST(InProcessModuleCache, ReadReadInvalidation) {
   EXPECT_EQ(Buf1->getBuffer().begin(), Buf2->getBuffer().begin());
   EXPECT_EQ(Buf1->getBuffer().end(), Buf2->getBuffer().end());
 }
+
+TEST(InProcessModuleCache, TimestampWrittenOncePerSession) {
+#ifndef _WIN32
+  // Logging (and thus this assertion) is only enabled on non-Windows platforms.
+  int FD;
+  llvm::SmallString<256> LogPath;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("m", "log", FD, LogPath));
+  { llvm::raw_fd_ostream ClaimFD(FD, /*shouldClose=*/true); }
+
+  {
+    ModuleCacheEntries Entries;
+    AtomicLineLogger Logger(LogPath);
+    std::shared_ptr<ModuleCache> ModCache =
+        makeInProcessModuleCache(Entries, Logger);
+
+    // Two workers validating the same shared module race to record its
+    // timestamp; only the first should record (and log) it.
+    ModCache->updateModuleTimestamp("A.pcm");
+    ModCache->updateModuleTimestamp("A.pcm");
+  }
+
+  auto Log = llvm::MemoryBuffer::getFile(LogPath);
+  ASSERT_TRUE(static_cast<bool>(Log));
+  EXPECT_EQ((*Log)->getBuffer().count("timestamp_write:"), 1u);
+
+  llvm::sys::fs::remove(LogPath);
+#endif
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/210587