Skip to content

Commit 43d042b

Browse files
qinkunbaoqinkunbao
authored
Revert "[clang][scan-deps] Add option to disable caching stat failures" (#145528)
Reverts #144000 First buildbot failure: https://lab.llvm.org/buildbot/#/builders/164/builds/11064
1 parent b581f9d commit 43d042b

File tree

9 files changed

+42
-132
lines changed

9 files changed

+42
-132
lines changed

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ namespace clang {
2323
namespace tooling {
2424
namespace dependencies {
2525

26-
class DependencyScanningService;
27-
2826
using DependencyDirectivesTy =
2927
SmallVector<dependency_directives_scan::Directive, 20>;
3028

@@ -351,7 +349,7 @@ class DependencyScanningWorkerFilesystem
351349
static const char ID;
352350

353351
DependencyScanningWorkerFilesystem(
354-
DependencyScanningService &Service,
352+
DependencyScanningFilesystemSharedCache &SharedCache,
355353
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
356354

357355
llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
@@ -437,7 +435,10 @@ class DependencyScanningWorkerFilesystem
437435
/// Returns entry associated with the unique ID in the shared cache or nullptr
438436
/// if none is found.
439437
const CachedFileSystemEntry *
440-
findSharedEntryByUID(llvm::vfs::Status Stat) const;
438+
findSharedEntryByUID(llvm::vfs::Status Stat) const {
439+
return SharedCache.getShardForUID(Stat.getUniqueID())
440+
.findEntryByUID(Stat.getUniqueID());
441+
}
441442

442443
/// Associates the given entry with the filename in the local cache and
443444
/// returns it.
@@ -451,14 +452,20 @@ class DependencyScanningWorkerFilesystem
451452
/// some. Otherwise, constructs new one with the given error code, associates
452453
/// it with the filename and returns the result.
453454
const CachedFileSystemEntry &
454-
getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC);
455+
getOrEmplaceSharedEntryForFilename(StringRef Filename, std::error_code EC) {
456+
return SharedCache.getShardForFilename(Filename)
457+
.getOrEmplaceEntryForFilename(Filename, EC);
458+
}
455459

456460
/// Returns entry associated with the filename in the shared cache if there is
457461
/// some. Otherwise, associates the given entry with the filename and returns
458462
/// it.
459463
const CachedFileSystemEntry &
460464
getOrInsertSharedEntryForFilename(StringRef Filename,
461-
const CachedFileSystemEntry &Entry);
465+
const CachedFileSystemEntry &Entry) {
466+
return SharedCache.getShardForFilename(Filename)
467+
.getOrInsertEntryForFilename(Filename, Entry);
468+
}
462469

463470
void printImpl(raw_ostream &OS, PrintType Type,
464471
unsigned IndentLevel) const override {
@@ -471,9 +478,8 @@ class DependencyScanningWorkerFilesystem
471478
/// VFS.
472479
bool shouldBypass(StringRef Path) const;
473480

474-
/// The service associated with this VFS.
475-
DependencyScanningService &Service;
476-
481+
/// The global cache shared between worker threads.
482+
DependencyScanningFilesystemSharedCache &SharedCache;
477483
/// The local cache is used by the worker thread to cache file system queries
478484
/// locally instead of querying the global cache every time.
479485
DependencyScanningFilesystemLocalCache LocalCache;

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ class DependencyScanningService {
8787
ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
8888
bool EagerLoadModules = false, bool TraceVFS = false,
8989
std::time_t BuildSessionTimestamp =
90-
llvm::sys::toTimeT(std::chrono::system_clock::now()),
91-
bool CacheNegativeStats = true);
90+
llvm::sys::toTimeT(std::chrono::system_clock::now()));
9291

9392
ScanningMode getMode() const { return Mode; }
9493

@@ -100,8 +99,6 @@ class DependencyScanningService {
10099

101100
bool shouldTraceVFS() const { return TraceVFS; }
102101

103-
bool shouldCacheNegativeStats() const { return CacheNegativeStats; }
104-
105102
DependencyScanningFilesystemSharedCache &getSharedCache() {
106103
return SharedCache;
107104
}
@@ -119,7 +116,6 @@ class DependencyScanningService {
119116
const bool EagerLoadModules;
120117
/// Whether to trace VFS accesses.
121118
const bool TraceVFS;
122-
const bool CacheNegativeStats;
123119
/// The global file system cache.
124120
DependencyScanningFilesystemSharedCache SharedCache;
125121
/// The global module cache entries.

‎clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
10-
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
1110
#include "llvm/Support/MemoryBuffer.h"
1211
#include "llvm/Support/Threading.h"
1312
#include <optional>
@@ -233,19 +232,19 @@ bool DependencyScanningWorkerFilesystem::shouldBypass(StringRef Path) const {
233232
}
234233

235234
DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
236-
DependencyScanningService &Service,
235+
DependencyScanningFilesystemSharedCache &SharedCache,
237236
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
238237
: llvm::RTTIExtends<DependencyScanningWorkerFilesystem,
239238
llvm::vfs::ProxyFileSystem>(std::move(FS)),
240-
Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
239+
SharedCache(SharedCache),
240+
WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
241241
updateWorkingDirForCacheLookup();
242242
}
243243

244244
const CachedFileSystemEntry &
245245
DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForUID(
246246
TentativeEntry TEntry) {
247-
auto &Shard =
248-
Service.getSharedCache().getShardForUID(TEntry.Status.getUniqueID());
247+
auto &Shard = SharedCache.getShardForUID(TEntry.Status.getUniqueID());
249248
return Shard.getOrEmplaceEntryForUID(TEntry.Status.getUniqueID(),
250249
std::move(TEntry.Status),
251250
std::move(TEntry.Contents));
@@ -256,44 +255,18 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough(
256255
StringRef Filename) {
257256
if (const auto *Entry = LocalCache.findEntryByFilename(Filename))
258257
return Entry;
259-
auto &Shard = Service.getSharedCache().getShardForFilename(Filename);
258+
auto &Shard = SharedCache.getShardForFilename(Filename);
260259
if (const auto *Entry = Shard.findEntryByFilename(Filename))
261260
return &LocalCache.insertEntryForFilename(Filename, *Entry);
262261
return nullptr;
263262
}
264263

265-
const CachedFileSystemEntry *
266-
DependencyScanningWorkerFilesystem::findSharedEntryByUID(
267-
llvm::vfs::Status Stat) const {
268-
return Service.getSharedCache()
269-
.getShardForUID(Stat.getUniqueID())
270-
.findEntryByUID(Stat.getUniqueID());
271-
}
272-
273-
const CachedFileSystemEntry &
274-
DependencyScanningWorkerFilesystem::getOrEmplaceSharedEntryForFilename(
275-
StringRef Filename, std::error_code EC) {
276-
return Service.getSharedCache()
277-
.getShardForFilename(Filename)
278-
.getOrEmplaceEntryForFilename(Filename, EC);
279-
}
280-
281-
const CachedFileSystemEntry &
282-
DependencyScanningWorkerFilesystem::getOrInsertSharedEntryForFilename(
283-
StringRef Filename, const CachedFileSystemEntry &Entry) {
284-
return Service.getSharedCache()
285-
.getShardForFilename(Filename)
286-
.getOrInsertEntryForFilename(Filename, Entry);
287-
}
288-
289264
llvm::ErrorOr<const CachedFileSystemEntry &>
290265
DependencyScanningWorkerFilesystem::computeAndStoreResult(
291266
StringRef OriginalFilename, StringRef FilenameForLookup) {
292267
llvm::ErrorOr<llvm::vfs::Status> Stat =
293268
getUnderlyingFS().status(OriginalFilename);
294269
if (!Stat) {
295-
if (!Service.shouldCacheNegativeStats())
296-
return Stat.getError();
297270
const auto &Entry =
298271
getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError());
299272
return insertLocalEntryForFilename(FilenameForLookup, Entry);
@@ -447,8 +420,7 @@ DependencyScanningWorkerFilesystem::getRealPath(const Twine &Path,
447420
return HandleCachedRealPath(*RealPath);
448421

449422
// If we have the result in the shared cache, cache it locally.
450-
auto &Shard =
451-
Service.getSharedCache().getShardForFilename(*FilenameForLookup);
423+
auto &Shard = SharedCache.getShardForFilename(*FilenameForLookup);
452424
if (const auto *ShardRealPath =
453425
Shard.findRealPathByFilename(*FilenameForLookup)) {
454426
const auto &RealPath = LocalCache.insertRealPathForFilename(

‎clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ using namespace dependencies;
1515
DependencyScanningService::DependencyScanningService(
1616
ScanningMode Mode, ScanningOutputFormat Format,
1717
ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS,
18-
std::time_t BuildSessionTimestamp, bool CacheNegativeStats)
18+
std::time_t BuildSessionTimestamp)
1919
: Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
2020
EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS),
21-
CacheNegativeStats(CacheNegativeStats),
2221
BuildSessionTimestamp(BuildSessionTimestamp) {}

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,8 @@ DependencyScanningWorker::DependencyScanningWorker(
611611

612612
switch (Service.getMode()) {
613613
case ScanningMode::DependencyDirectivesScan:
614-
DepFS = new DependencyScanningWorkerFilesystem(Service, FS);
614+
DepFS =
615+
new DependencyScanningWorkerFilesystem(Service.getSharedCache(), FS);
615616
BaseFS = DepFS;
616617
break;
617618
case ScanningMode::CanonicalPreprocessing:

‎clang/tools/clang-scan-deps/ClangScanDeps.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ static ScanningOutputFormat Format = ScanningOutputFormat::Make;
8585
static ScanningOptimizations OptimizeArgs;
8686
static std::string ModuleFilesDir;
8787
static bool EagerLoadModules;
88-
static bool CacheNegativeStats = true;
8988
static unsigned NumThreads = 0;
9089
static std::string CompilationDB;
9190
static std::optional<std::string> ModuleName;
@@ -192,8 +191,6 @@ static void ParseArgs(int argc, char **argv) {
192191

193192
EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
194193

195-
CacheNegativeStats = !Args.hasArg(OPT_no_cache_negative_stats);
196-
197194
if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
198195
StringRef S{A->getValue()};
199196
if (!llvm::to_integer(S, NumThreads, 0)) {
@@ -1083,9 +1080,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
10831080
});
10841081
};
10851082

1086-
DependencyScanningService Service(
1087-
ScanMode, Format, OptimizeArgs, EagerLoadModules, /*TraceVFS=*/Verbose,
1088-
llvm::sys::toTimeT(std::chrono::system_clock::now()), CacheNegativeStats);
1083+
DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
1084+
EagerLoadModules, /*TraceVFS=*/Verbose);
10891085

10901086
llvm::Timer T;
10911087
T.startTimer();

‎clang/tools/clang-scan-deps/Opts.td‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ defm module_files_dir : Eq<"module-files-dir",
2222

2323
def optimize_args_EQ : CommaJoined<["-", "--"], "optimize-args=">, HelpText<"Which command-line arguments of modules to optimize">;
2424
def eager_load_pcm : F<"eager-load-pcm", "Load PCM files eagerly (instead of lazily on import)">;
25-
def no_cache_negative_stats : F<"no-cache-negative-stats", "Don't cache stat failures">;
2625

2726
def j : Arg<"j", "Number of worker threads to use (default: use all concurrent threads)">;
2827

‎clang/unittests/Tooling/DependencyScanning/DependencyScannerTest.cpp‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -384,55 +384,3 @@ TEST(DependencyScanner, ScanDepsWithDiagConsumer) {
384384
EXPECT_TRUE(DiagConsumer.Finished);
385385
}
386386
}
387-
388-
TEST(DependencyScanner, NoNegativeCache) {
389-
StringRef CWD = "/root";
390-
391-
auto VFS = new llvm::vfs::InMemoryFileSystem();
392-
VFS->setCurrentWorkingDirectory(CWD);
393-
auto Sept = llvm::sys::path::get_separator();
394-
std::string HeaderPath =
395-
std::string(llvm::formatv("{0}root{0}header.h", Sept));
396-
std::string Test0Path =
397-
std::string(llvm::formatv("{0}root{0}test0.cpp", Sept));
398-
std::string Test1Path =
399-
std::string(llvm::formatv("{0}root{0}test1.cpp", Sept));
400-
401-
VFS->addFile(Test0Path, 0,
402-
llvm::MemoryBuffer::getMemBuffer(
403-
"#if __has_include(\"header.h\")\n#endif"));
404-
VFS->addFile(Test1Path, 0,
405-
llvm::MemoryBuffer::getMemBuffer("#include \"header.h\""));
406-
407-
DependencyScanningService Service(
408-
ScanningMode::DependencyDirectivesScan, ScanningOutputFormat::Make,
409-
ScanningOptimizations::All, false, false,
410-
llvm::sys::toTimeT(std::chrono::system_clock::now()), false);
411-
DependencyScanningTool ScanTool(Service, VFS);
412-
413-
std::vector<std::string> CommandLine0 = {"clang",
414-
"-target",
415-
"x86_64-apple-macosx10.7",
416-
"-c",
417-
"test0.cpp",
418-
"-o"
419-
"test0.cpp.o"};
420-
std::vector<std::string> CommandLine1 = {"clang",
421-
"-target",
422-
"x86_64-apple-macosx10.7",
423-
"-c",
424-
"test1.cpp",
425-
"-o"
426-
"test1.cpp.o"};
427-
428-
std::string Result;
429-
ASSERT_THAT_ERROR(
430-
ScanTool.getDependencyFile(CommandLine0, CWD).moveInto(Result),
431-
llvm::Succeeded());
432-
433-
VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(""));
434-
435-
ASSERT_THAT_ERROR(
436-
ScanTool.getDependencyFile(CommandLine1, CWD).moveInto(Result),
437-
llvm::Succeeded());
438-
}

‎clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
10-
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
1110
#include "llvm/ADT/SmallString.h"
1211
#include "llvm/Support/VirtualFileSystem.h"
1312
#include "gtest/gtest.h"
@@ -20,10 +19,9 @@ TEST(DependencyScanningWorkerFilesystem, CacheStatusFailures) {
2019
auto InstrumentingFS =
2120
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
2221

23-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
24-
ScanningOutputFormat::Make);
25-
DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
26-
DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
22+
DependencyScanningFilesystemSharedCache SharedCache;
23+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
24+
DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
2725

2826
DepFS.status("/foo.c");
2927
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 1u);
@@ -47,10 +45,9 @@ TEST(DependencyScanningFilesystem, CacheGetRealPath) {
4745
auto InstrumentingFS =
4846
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
4947

50-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
51-
ScanningOutputFormat::Make);
52-
DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
53-
DependencyScanningWorkerFilesystem DepFS2(Service, InstrumentingFS);
48+
DependencyScanningFilesystemSharedCache SharedCache;
49+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
50+
DependencyScanningWorkerFilesystem DepFS2(SharedCache, InstrumentingFS);
5451

5552
{
5653
llvm::SmallString<128> Result;
@@ -83,9 +80,8 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
8380
InMemoryFS->addFile("/foo.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
8481
InMemoryFS->addFile("/bar.c", 0, llvm::MemoryBuffer::getMemBuffer(""));
8582

86-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
87-
ScanningOutputFormat::Make);
88-
DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
83+
DependencyScanningFilesystemSharedCache SharedCache;
84+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
8985

9086
// Success.
9187
{
@@ -137,9 +133,8 @@ TEST(DependencyScanningFilesystem, CacheStatOnExists) {
137133
InMemoryFS->setCurrentWorkingDirectory("/");
138134
InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
139135
InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
140-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
141-
ScanningOutputFormat::Make);
142-
DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
136+
DependencyScanningFilesystemSharedCache SharedCache;
137+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
143138

144139
DepFS.status("/foo");
145140
DepFS.status("/foo");
@@ -161,9 +156,8 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
161156
auto InstrumentingFS =
162157
llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(InMemoryFS);
163158

164-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
165-
ScanningOutputFormat::Make);
166-
DependencyScanningWorkerFilesystem DepFS(Service, InstrumentingFS);
159+
DependencyScanningFilesystemSharedCache SharedCache;
160+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);
167161

168162
DepFS.status("/dir");
169163
DepFS.status("/dir");
@@ -189,9 +183,8 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
189183
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
190184
InMemoryFS->setCurrentWorkingDirectory("/");
191185

192-
DependencyScanningService Service(ScanningMode::DependencyDirectivesScan,
193-
ScanningOutputFormat::Make);
194-
DependencyScanningWorkerFilesystem DepFS(Service, InMemoryFS);
186+
DependencyScanningFilesystemSharedCache SharedCache;
187+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
195188

196189
bool Path1Exists = DepFS.exists("/path1.suffix");
197190
EXPECT_EQ(Path1Exists, false);
@@ -204,7 +197,7 @@ TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
204197
EXPECT_EQ(Path1Exists, false);
205198

206199
std::vector<llvm::StringRef> InvalidPaths =
207-
Service.getSharedCache().getInvalidNegativeStatCachedPaths(*InMemoryFS);
200+
SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS);
208201

209202
EXPECT_EQ(InvalidPaths.size(), 1u);
210203
ASSERT_STREQ("/path1.suffix", InvalidPaths[0].str().c_str());

0 commit comments

Comments
 (0)