Author: Qinkun Bao Date: 2025-06-24T11:12:05-04:00 New Revision: 43d042b350af8ee8c7401d6b102df68d6c176b5a
URL: https://github.com/llvm/llvm-project/commit/43d042b350af8ee8c7401d6b102df68d6c176b5a DIFF: https://github.com/llvm/llvm-project/commit/43d042b350af8ee8c7401d6b102df68d6c176b5a.diff LOG: Revert "[clang][scan-deps] Add option to disable caching stat failures" (#145528) Reverts llvm/llvm-project#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064 Added: Modified: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/tools/clang-scan-deps/ClangScanDeps.cpp clang/tools/clang-scan-deps/Opts.td clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index da83220babea3..a20a89a4c2b76 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -23,8 +23,6 @@ namespace clang { namespace tooling { namespace dependencies { -class DependencyScanningService; - using DependencyDirectivesTy = SmallVector<dependency_directives_scan::Directive, 20>; @@ -351,7 +349,7 @@ class DependencyScanningWorkerFilesystem static const char ID; DependencyScanningWorkerFilesystem( - DependencyScanningService &Service, + DependencyScanningFilesystemSharedCache &SharedCache, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override; @@ -437,7 +435,10 @@ class DependencyScanningWorkerFilesystem /// Returns entry associated with the unique ID in the shared cache or nullptr /// if none is found. const CachedFileSystemEntry * - findSharedEntryByUID(llvm::vfs::Status Stat) const; + findSharedEntryByUID(llvm::vfs::Status Stat) const { + return SharedCache.getShardForUID(Stat.getUniqueID()) + .findEntryByUID(Stat.getUniqueID()); + } /// Associates the given entry with the filename in the local cache and /// returns it. @@ -451,14 +452,20 @@ class DependencyScanningWorkerFilesystem /// some. Otherwise, constructs new one with the given error code, associates /// it with the filename and returns the result. const CachedFileSystemEntry & - getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC); + getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC) { + return SharedCache.getShardForFilename(Filename) + .getOrEmplaceEntryForFilename(Filename, EC); + } /// Returns entry associated with the filename in the shared cache if there is /// some. Otherwise, associates the given entry with the filename and returns /// it. const CachedFileSystemEntry & getOrInsertSharedEntryForFilename(StringRef Filename, - const CachedFileSystemEntry &Entry); + const CachedFileSystemEntry &Entry) { + return SharedCache.getShardForFilename(Filename) + .getOrInsertEntryForFilename(Filename, Entry); + } void printImpl(raw_ostream &OS, PrintType Type, unsigned IndentLevel) const override { @@ -471,9 +478,8 @@ class DependencyScanningWorkerFilesystem /// VFS. bool shouldBypass(StringRef Path) const; - /// The service associated with this VFS. - DependencyScanningService &Service; - + /// The global cache shared between worker threads. + DependencyScanningFilesystemSharedCache &SharedCache; /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index ceaf3c2279e7f..4e97c7bc9f36e 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -87,8 +87,7 @@ class DependencyScanningService { ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default, bool EagerLoadModules = false, bool TraceVFS = false, std::time_t BuildSessionTimestamp = - llvm::sys::toTimeT(std::chrono::system_clock::now()), - bool CacheNegativeStats = true); + llvm::sys::toTimeT(std::chrono::system_clock::now())); ScanningMode getMode() const { return Mode; } @@ -100,8 +99,6 @@ class DependencyScanningService { bool shouldTraceVFS() const { return TraceVFS; } - bool shouldCacheNegativeStats() const { return CacheNegativeStats; } - DependencyScanningFilesystemSharedCache &getSharedCache() { return SharedCache; } @@ -119,7 +116,6 @@ class DependencyScanningService { const bool EagerLoadModules; /// Whether to trace VFS accesses. const bool TraceVFS; - const bool CacheNegativeStats; /// The global file system cache. DependencyScanningFilesystemSharedCache SharedCache; /// The global module cache entries. diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 2868522f80018..140833050f4e9 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" -#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Threading.h" #include <optional> @@ -233,19 +232,19 @@ bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const { } DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem( - DependencyScanningService &Service, + DependencyScanningFilesystemSharedCache &SharedCache, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) : llvm::RTTIExtends<DependencyScanningWorkerFilesystem, llvm::vfs::ProxyFileSystem>(std::move(FS)), - Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument) { + SharedCache(SharedCache), + WorkingDirForCacheLookup(llvm::errc::invalid_argument) { updateWorkingDirForCacheLookup(); } const CachedFileSystemEntry & DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID( TentativeEntry TEntry) { - auto &Shard = - Service.getSharedCache().getShardForUID(TEntry.Status.getUniqueID()); + auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID()); return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(), std::move(TEntry.Status), std::move(TEntry.Contents)); @@ -256,44 +255,18 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( StringRef Filename) { if (const auto *Entry = LocalCache.findEntryByFilename(Filename)) return Entry; - auto &Shard = Service.getSharedCache().getShardForFilename(Filename); + auto &Shard = SharedCache.getShardForFilename(Filename); if (const auto *Entry = Shard.findEntryByFilename(Filename)) return &LocalCache.insertEntryForFilename(Filename, *Entry); return nullptr; } -const CachedFileSystemEntry * -DependencyScanningWorkerFilesystem::findSharedEntryByUID( - llvm::vfs::Status Stat) const { - return Service.getSharedCache() - .getShardForUID(Stat.getUniqueID()) - .findEntryByUID(Stat.getUniqueID()); -} - -const CachedFileSystemEntry & -DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForFilename( - StringRef Filename, std::error_code EC) { - return Service.getSharedCache() - .getShardForFilename(Filename) - .getOrEmplaceEntryForFilename(Filename, EC); -} - -const CachedFileSystemEntry & -DependencyScanningWorkerFilesystem::getOrInsertSharedEntryForFilename( - StringRef Filename, const CachedFileSystemEntry &Entry) { - return Service.getSharedCache() - .getShardForFilename(Filename) - .getOrInsertEntryForFilename(Filename, Entry); -} - llvm::ErrorOr<const CachedFileSystemEntry &> DependencyScanningWorkerFilesystem::computeAndStoreResult( StringRef OriginalFilename, StringRef FilenameForLookup) { llvm::ErrorOr<llvm::vfs::Status> Stat = getUnderlyingFS().status(OriginalFilename); if (!Stat) { - if (!Service.shouldCacheNegativeStats()) - return Stat.getError(); const auto &Entry = getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); return insertLocalEntryForFilename(FilenameForLookup, Entry); @@ -447,8 +420,7 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path, return HandleCachedRealPath(*RealPath); // If we have the result in the shared cache, cache it locally. - auto &Shard = - Service.getSharedCache().getShardForFilename(*FilenameForLookup); + auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup); if (const auto *ShardRealPath = Shard.findRealPathByFilename(*FilenameForLookup)) { const auto &RealPath = LocalCache.insertRealPathForFilename( diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp index c2f3cdbb02e37..7f40c99f07287 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp @@ -15,8 +15,7 @@ using namespace dependencies; DependencyScanningService::DependencyScanningService( ScanningMode Mode, ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS, - std::time_t BuildSessionTimestamp, bool CacheNegativeStats) + std::time_t BuildSessionTimestamp) : Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS), - CacheNegativeStats(CacheNegativeStats), BuildSessionTimestamp(BuildSessionTimestamp) {} diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index d9820ca3c584e..9bd85479d9810 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -611,7 +611,8 @@ DependencyScanningWorker::DependencyScanningWorker( switch (Service.getMode()) { case ScanningMode::DependencyDirectivesScan: - DepFS = new DependencyScanningWorkerFilesystem(Service, FS); + DepFS = + new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS); BaseFS = DepFS; break; case ScanningMode::CanonicalPreprocessing: diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index ce0770a51d65f..921ba7aadd67d 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -85,7 +85,6 @@ static ScanningOutputFormat Format = ScanningOutputFormat::Make; static ScanningOptimizations OptimizeArgs; static std::string ModuleFilesDir; static bool EagerLoadModules; -static bool CacheNegativeStats = true; static unsigned NumThreads = 0; static std::string CompilationDB; static std::optional<std::string> ModuleName; @@ -192,8 +191,6 @@ static void ParseArgs(int argc, char **argv) { EagerLoadModules = Args.hasArg(OPT_eager_load_pcm); - CacheNegativeStats = !Args.hasArg(OPT_no_cache_negative_stats); - if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) { StringRef S{A->getValue()}; if (!llvm::to_integer(S, NumThreads, 0)) { @@ -1083,9 +1080,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) { }); }; - DependencyScanningService Service( - ScanMode, Format, OptimizeArgs, EagerLoadModules, /*TraceVFS=*/Verbose, - llvm::sys::toTimeT(std::chrono::system_clock::now()), CacheNegativeStats); + DependencyScanningService Service(ScanMode, Format, OptimizeArgs, + EagerLoadModules, /*TraceVFS=*/Verbose); llvm::Timer T; T.startTimer(); diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td index 582ae60851e1e..9cccbb3aaf0c8 100644 --- a/clang/tools/clang-scan-deps/Opts.td +++ b/clang/tools/clang-scan-deps/Opts.td @@ -22,7 +22,6 @@ defm module_files_dir : Eq<"module-files-dir", def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">; def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">; -def no_cache_negative_stats : F<"no-cache-negative-stats", "Don't cache stat failures">; def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">; diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp index d194b2877ad8f..683d9070b1dcf 100644 --- a/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp @@ -384,55 +384,3 @@ TEST(DependencyScanner, ScanDepsWithDiagConsumer) { EXPECT_TRUE(DiagConsumer.Finished); } } - -TEST(DependencyScanner, NoNegativeCache) { - StringRef CWD = "/root"; - - auto VFS = new llvm::vfs::InMemoryFileSystem(); - VFS->setCurrentWorkingDirectory(CWD); - auto Sept = llvm::sys::path::get_separator(); - std::string HeaderPath = - std::string(llvm::formatv("{0}root{0}header.h", Sept)); - std::string Test0Path = - std::string(llvm::formatv("{0}root{0}test0.cpp", Sept)); - std::string Test1Path = - std::string(llvm::formatv("{0}root{0}test1.cpp", Sept)); - - VFS->addFile(Test0Path, 0, - llvm::MemoryBuffer::getMemBuffer( - "#if __has_include(\"header.h\")\n#endif")); - VFS->addFile(Test1Path, 0, - llvm::MemoryBuffer::getMemBuffer("#include \"header.h\"")); - - DependencyScanningService Service( - ScanningMode::DependencyDirectivesScan, ScanningOutputFormat::Make, - ScanningOptimizations::All, false, false, - llvm::sys::toTimeT(std::chrono::system_clock::now()), false); - DependencyScanningTool ScanTool(Service, VFS); - - std::vector<std::string> CommandLine0 = {"clang", - "-target", - "x86_64-apple-macosx10.7", - "-c", - "test0.cpp", - "-o" - "test0.cpp.o"}; - std::vector<std::string> CommandLine1 = {"clang", - "-target", - "x86_64-apple-macosx10.7", - "-c", - "test1.cpp", - "-o" - "test1.cpp.o"}; - - std::string Result; - ASSERT_THAT_ERROR( - ScanTool.getDependencyFile(CommandLine0, CWD).moveInto(Result), - llvm::Succeeded()); - - VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer("")); - - ASSERT_THAT_ERROR( - ScanTool.getDependencyFile(CommandLine1, CWD).moveInto(Result), - llvm::Succeeded()); -} diff --git a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp index a68ea72d3816c..7420743c97a2a 100644 --- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp +++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" -#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/VirtualFileSystem.h" #include "gtest/gtest.h" @@ -20,10 +19,9 @@ TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) { auto InstrumentingFS = llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS); - DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS); + DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS); DepFS.status("/foo.c"); EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u); @@ -47,10 +45,9 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) { auto InstrumentingFS = llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS); - DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS); + DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS); { llvm::SmallString<128> Result; @@ -83,9 +80,8 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) { InMemoryFS->addFile("/foo.c", 0, llvm::MemoryBuffer::getMemBuffer("")); InMemoryFS->addFile("/bar.c", 0, llvm::MemoryBuffer::getMemBuffer("")); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS); // Success. { @@ -137,9 +133,8 @@ TEST(DependencyScanningFilesystem, CacheStatOnExists) { InMemoryFS->setCurrentWorkingDirectory("/"); InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer("")); InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer("")); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS); DepFS.status("/foo"); DepFS.status("/foo"); @@ -161,9 +156,8 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) { auto InstrumentingFS = llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS); DepFS.status("/dir"); DepFS.status("/dir"); @@ -189,9 +183,8 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) { auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); InMemoryFS->setCurrentWorkingDirectory("/"); - DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, - ScanningOutputFormat::Make); - DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS); + DependencyScanningFilesystemSharedCache SharedCache; + DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS); bool Path1Exists = DepFS.exists("/path1.suffix"); EXPECT_EQ(Path1Exists, false); @@ -204,7 +197,7 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) { EXPECT_EQ(Path1Exists, false); std::vector<llvm::StringRef> InvalidPaths = - Service.getSharedCache().getInvalidNegativeStatCachedPaths(*InMemoryFS); + SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS); EXPECT_EQ(InvalidPaths.size(), 1u); ASSERT_STREQ("/path1.suffix", InvalidPaths[0].str().c_str()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
