Skip to content

Commit

Permalink
[clang][deps] Overload Filesystem::exists in `DependencyScanningFil…
Browse files Browse the repository at this point in the history
…esystem` to have it use cached `status` (#88152)

As-is, calls to `exists()` fallback on the implementation in
`ProxyFileSystem::exists` which explicitly calls out to the underlying
`FS`, which for the `DependencyScanningFilesystem` (overlay) is the real
underlying filesystem.

Instead, directly overloading `exists` allows us to have it rely on the
cached `status` behavior used elsewhere by the
`DependencyScanningFilesystem`.
  • Loading branch information
artemcm committed Apr 12, 2024
1 parent db8e182 commit 779ba60
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,10 @@ class DependencyScanningWorkerFilesystem
/// false if not (i.e. this entry is not a file or its scan fails).
bool ensureDirectiveTokensArePopulated(EntryRef Entry);

/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
bool exists(const Twine &Path) override;

private:
/// For a filename that's not yet associated with any entry in the caches,
/// uses the underlying filesystem to either look up the entry based in the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,17 @@ DependencyScanningWorkerFilesystem::status(const Twine &Path) {
return Result->getStatus();
}

bool DependencyScanningWorkerFilesystem::exists(const Twine &Path) {
// While some VFS overlay filesystems may implement more-efficient
// mechanisms for `exists` queries, `DependencyScanningWorkerFilesystem`
// typically wraps `RealFileSystem` which does not specialize `exists`,
// so it is not likely to benefit from such optimizations. Instead,
// it is more-valuable to have this query go through the
// cached-`status` code-path of the `DependencyScanningWorkerFilesystem`.
llvm::ErrorOr<llvm::vfs::Status> Status = status(Path);
return Status && Status->exists();
}

namespace {

/// The VFS that is used by clang consumes the \c CachedFileSystemEntry using
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct InstrumentingFilesystem
: llvm::RTTIExtends<InstrumentingFilesystem, llvm::vfs::ProxyFileSystem> {
unsigned NumStatusCalls = 0;
unsigned NumGetRealPathCalls = 0;
unsigned NumExistsCalls = 0;

using llvm::RTTIExtends<InstrumentingFilesystem,
llvm::vfs::ProxyFileSystem>::RTTIExtends;
Expand All @@ -32,6 +33,11 @@ struct InstrumentingFilesystem
++NumGetRealPathCalls;
return ProxyFileSystem::getRealPath(Path, Output);
}

bool exists(const llvm::Twine &Path) override {
++NumExistsCalls;
return ProxyFileSystem::exists(Path);
}
};
} // namespace

Expand Down Expand Up @@ -147,3 +153,24 @@ TEST(DependencyScanningFilesystem, RealPathAndStatusInvariants) {
DepFS.status("/bar");
}
}

TEST(DependencyScanningFilesystem, CacheStatOnExists) {
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
auto InstrumentingFS =
llvm::makeIntrusiveRefCnt<InstrumentingFilesystem>(InMemoryFS);
InMemoryFS->setCurrentWorkingDirectory("/");
InMemoryFS->addFile("/foo", 0, llvm::MemoryBuffer::getMemBuffer(""));
InMemoryFS->addFile("/bar", 0, llvm::MemoryBuffer::getMemBuffer(""));
DependencyScanningFilesystemSharedCache SharedCache;
DependencyScanningWorkerFilesystem DepFS(SharedCache, InstrumentingFS);

DepFS.status("/foo");
DepFS.status("/foo");
DepFS.status("/bar");
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u);

DepFS.exists("/foo");
DepFS.exists("/bar");
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u);
EXPECT_EQ(InstrumentingFS->NumExistsCalls, 0u);
}

0 comments on commit 779ba60

Please sign in to comment.