Revert "[clang][scan-deps] Add option to disable caching stat failures" #145528
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Qinkun Bao (qinkunbao) ChangesReverts llvm/llvm-project#144000 First buildbot failure: Full diff: https://github.com/llvm/llvm-project/pull/145528.diff 9 Files Affected:
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());
|
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
…s" (llvm#145528) Reverts llvm#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
…s" (llvm#145528) Reverts llvm#144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #144000
First buildbot failure:
https://lab.llvm.org/buildbot/#/builders/164/builds/11064