https://github.com/atassis updated https://github.com/llvm/llvm-project/pull/210587
>From 7e56c04cfe2037d1c585ab0fe6312ca65774b026 Mon Sep 17 00:00:00 2001 From: Taimuraz Kaitmazov <[email protected]> Date: Sun, 19 Jul 2026 12:18:36 +0300 Subject: [PATCH 1/2] [clang][modules] Write each module's timestamp once per build session 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. --- .../InProcessModuleCache.cpp | 9 ++++-- .../InProcessModuleCacheTest.cpp | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) 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..1a5aa23ef297c 100644 --- a/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp +++ b/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp @@ -74,3 +74,33 @@ 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 +} >From 0f0f6c604075f3387f1e871650b9dd4a7f605343 Mon Sep 17 00:00:00 2001 From: Taimuraz Kaitmazov <[email protected]> Date: Tue, 21 Jul 2026 20:21:51 +0300 Subject: [PATCH 2/2] [clang][ClangScanDeps] Relax logging-two-threads for duplicate validation Drop the module-cache mutex from the previous revision and fix the test instead, per review: the double timestamp write is already race-free (the field is atomic) and the lock only deduplicated the log line. The module timestamp records when a worker last validated a module's inputs in the build session. Under -fmodules-validate-once-per-build-session two workers can both notice the shared module A is unvalidated and each validate and record it before the other's write lands, so A's timestamp is written once or twice depending on the interleaving. The test asserted exactly one write per module, which made it flaky. Assert the real invariant instead: the shared module A is written once or twice and the single-TU module B exactly once. Compile and pcm write stay exactly once (the in-memory cache builds each module once) and the per-module event ordering is still checked by ASEQ/BSEQ. --- .../InProcessModuleCache.cpp | 9 ++---- .../test/ClangScanDeps/logging-two-threads.c | 16 +++++++--- .../InProcessModuleCacheTest.cpp | 30 ------------------- 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index 3608a18dd7794..bc6ff88c57323 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -128,15 +128,10 @@ class InProcessModuleCache : public ModuleCache { void updateModuleTimestamp(StringRef Filename) override { // Note: This essentially replaces FS contention with mutex contention. - ModuleCacheEntry &Entry = getOrCreateEntry(Filename); - std::lock_guard<std::mutex> Lock(Entry.Mutex); + auto &Timestamp = getOrCreateEntry(Filename).Timestamp; - // 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/test/ClangScanDeps/logging-two-threads.c b/clang/test/ClangScanDeps/logging-two-threads.c index a56117f67ba09..e3173deaddcf6 100644 --- a/clang/test/ClangScanDeps/logging-two-threads.c +++ b/clang/test/ClangScanDeps/logging-two-threads.c @@ -19,14 +19,22 @@ // TU2: starting scanning command:{{.*}}tu2.c // TU2: finished scanning command:{{.*}}tu2.c -// Each module is compiled once, its pcm written once, and its timestamp written -// once, regardless of which thread wins the race to build the shared module A. +// Each module is compiled once and its pcm written once, regardless of which +// thread wins the race to build the shared module A. // RUN: grep -c "pcm_compile:" %t/scan.log | FileCheck %s --check-prefix=COMPILES // RUN: grep -c "pcm_write:" %t/scan.log | FileCheck %s --check-prefix=PCMWRITES -// RUN: grep -c "timestamp_write:" %t/scan.log | FileCheck %s --check-prefix=TSWRITES // COMPILES: {{^2$}} // PCMWRITES: {{^2$}} -// TSWRITES: {{^2$}} + +// The timestamp records when a worker last validated a module's inputs in this +// build session. Both threads can notice the shared module A is unvalidated +// before either records it, so each may validate and record it: A's timestamp +// is written once or twice. B (imported by a single TU) is always written once. +// The per-module ordering is checked by ASEQ/BSEQ below. +// RUN: grep -c "timestamp_write:.*A-" %t/scan.log | FileCheck %s --check-prefix=TSA +// RUN: grep -c "timestamp_write:.*B-" %t/scan.log | FileCheck %s --check-prefix=TSB +// TSA: {{^[12]$}} +// TSB: {{^1$}} // Verify A's pcm is written before B's pcm. // RUN: grep "pcm_write:" %t/scan.log | FileCheck %s --check-prefix=WRITEORDER diff --git a/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp b/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp index 1a5aa23ef297c..5b667a4989f50 100644 --- a/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp +++ b/clang/unittests/DependencyScanning/InProcessModuleCacheTest.cpp @@ -74,33 +74,3 @@ 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 -} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
