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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
