https://github.com/akyrtzi created https://github.com/llvm/llvm-project/pull/234393
Since `DependencyScanningFilesystem` resolves FS queries through a global shared cache, the VFS it receives cannot be a custom VFS for different compiler invocations. To address this add an optional `LocalFS` whose queries will be cached by the worker only, without going through the shared cache. A caller can use this to pass a VFS-per-invocation for dependency scanning purposes. >From a930d0e48646b1ce165e5bfee54612145a1d30bc Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis <[email protected]> Date: Wed, 21 Oct 2026 14:41:51 -0700 Subject: [PATCH] [clang] Enhance `DependencyScanningFilesystem` to optionally accept a VFS for non-shared cache queries Since `DependencyScanningFilesystem` resolves FS queries through a global shared cache, the VFS it receives cannot be a custom VFS for different compiler invocations. To address this add an optional `LocalFS` whose queries will be cached by the worker only, without going through the shared cache. A caller can use this to pass a VFS-per-invocation for dependency scanning purposes. --- .../DependencyScanningFilesystem.h | 32 +++++++- .../DependencyScanningWorker.h | 4 +- .../clang/Tooling/DependencyScanningTool.h | 6 +- .../DependencyScanningFilesystem.cpp | 77 ++++++++++++++----- .../DependencyScanningWorker.cpp | 5 +- .../DependencyScanningFilesystemTest.cpp | 60 +++++++++++++++ 6 files changed, 158 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h index ec95c5b38f102f..7b0bb0e25bc7c1 100644 --- a/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/DependencyScanning/DependencyScanningFilesystem.h @@ -323,7 +323,8 @@ class DependencyScanningWorkerFilesystem DependencyScanningWorkerFilesystem( DependencyScanningService &Service, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS); + IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS = nullptr); llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override; llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>> @@ -379,6 +380,13 @@ class DependencyScanningWorkerFilesystem resolveUIDThroughSharedCache(StringRef OriginalFilename, const llvm::vfs::Status &Stat); + /// Resolves the cache entry through the local VFS and without going through + /// the global shared cache. Only relevant if \c LocalFS is non-null and + /// \p OriginalFilename exists in \c LocalFS. + const CachedFileSystemEntry * + createCachedFSEntryForLocal(StringRef OriginalFilename, + const llvm::vfs::Status &Stat); + /// Represents a filesystem entry that has been stat-ed (and potentially read) /// and that's about to be inserted into the cache as `CachedFileSystemEntry`. struct TentativeEntry { @@ -392,7 +400,8 @@ class DependencyScanningWorkerFilesystem /// Reads file at the given path. Enforces consistency between the file size /// in status and size of read contents. - llvm::ErrorOr<TentativeEntry> readFile(StringRef Filename); + static llvm::ErrorOr<TentativeEntry> readFile(StringRef Filename, + llvm::vfs::FileSystem &FS); void printImpl(raw_ostream &OS, PrintType Type, unsigned IndentLevel) const override { @@ -421,6 +430,25 @@ class DependencyScanningWorkerFilesystem /// using them for cache lookups. llvm::ErrorOr<std::string> WorkingDirForCacheLookup; + /// If non-null it is used to resolve files without going through the global + /// shared cache. This is useful if the caller wants to use a specific VFS + /// for a compiler invocation + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS; + + /// The FS that is used for queries going through the global shared cache. + IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS; + + /// The backing storage for cached entries resolved from local FS only. + llvm::SpecificBumpPtrAllocator<CachedFileSystemEntry> LocalEntryStorage; + + /// The backing storage for cached contents resolved from local FS only. + llvm::SpecificBumpPtrAllocator<CachedFileContents> LocalContentsStorage; + + static const CachedFileSystemEntry *createSharedEntry( + llvm::ErrorOr<TentativeEntry> &Entry, + llvm::SpecificBumpPtrAllocator<CachedFileContents> &ContentsStorage, + llvm::SpecificBumpPtrAllocator<CachedFileSystemEntry> &EntryStorage); + void updateWorkingDirForCacheLookup(); llvm::ErrorOr<StringRef> diff --git a/clang/include/clang/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h index 2c5c7fbb8f9a01..a7a9bcad7f81a8 100644 --- a/clang/include/clang/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/DependencyScanning/DependencyScanningWorker.h @@ -47,7 +47,9 @@ class DependencyScanningWorker { /// Construct a dependency scanning worker. /// /// @param Service The parent service. Must outlive the worker. - DependencyScanningWorker(DependencyScanningService &Service); + DependencyScanningWorker( + DependencyScanningService &Service, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS = nullptr); ~DependencyScanningWorker(); diff --git a/clang/include/clang/Tooling/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanningTool.h index 53a6615ab88c1d..5fe73d5d5d0bed 100644 --- a/clang/include/clang/Tooling/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanningTool.h @@ -35,8 +35,10 @@ class DependencyScanningTool { /// Construct a dependency scanning tool. /// /// @param Service The parent service. Must outlive the tool. - DependencyScanningTool(dependencies::DependencyScanningService &Service) - : Worker(Service) {} + DependencyScanningTool( + dependencies::DependencyScanningService &Service, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS = nullptr) + : Worker(Service, LocalFS) {} /// Print out the dependency information into a string using the dependency /// file format that is specified in the options (-MD is the default) and diff --git a/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp index 3b00411b7d2db7..c4281b82738863 100644 --- a/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/DependencyScanning/DependencyScanningFilesystem.cpp @@ -16,9 +16,10 @@ using namespace clang; using namespace dependencies; llvm::ErrorOr<DependencyScanningWorkerFilesystem::TentativeEntry> -DependencyScanningWorkerFilesystem::readFile(StringRef Filename) { +DependencyScanningWorkerFilesystem::readFile(StringRef Filename, + llvm::vfs::FileSystem &FS) { // Load the file and its content from the file system. - auto MaybeFile = getUnderlyingFS().openFileForRead(Filename); + auto MaybeFile = FS.openFileForRead(Filename); if (!MaybeFile) return MaybeFile.getError(); auto File = std::move(*MaybeFile); @@ -223,15 +224,48 @@ const CachedRealPath &DependencyScanningFilesystemSharedCache::CacheShard:: return *StoredRealPath; } +static IntrusiveRefCntPtr<llvm::vfs::FileSystem> +combineGlobalAndLocalFS(IntrusiveRefCntPtr<llvm::vfs::FileSystem> GlobalFS, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS) { + if (LocalFS) { + auto OverlayVFS = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>( + std::move(LocalFS)); + OverlayVFS->pushOverlay(std::move(GlobalFS)); + GlobalFS = std::move(OverlayVFS); + } + return GlobalFS; +} + DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem( DependencyScanningService &Service, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) + IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS) : llvm::RTTIExtends<DependencyScanningWorkerFilesystem, - llvm::vfs::ProxyFileSystem>(std::move(FS)), - Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument) { + llvm::vfs::ProxyFileSystem>( + combineGlobalAndLocalFS(FS, LocalFS)), + Service(Service), WorkingDirForCacheLookup(llvm::errc::invalid_argument), + LocalFS(std::move(LocalFS)), BaseFS(FS) { updateWorkingDirForCacheLookup(); } +const CachedFileSystemEntry * +DependencyScanningWorkerFilesystem::createSharedEntry( + llvm::ErrorOr<TentativeEntry> &TEntry, + llvm::SpecificBumpPtrAllocator<CachedFileContents> &ContentsStorage, + llvm::SpecificBumpPtrAllocator<CachedFileSystemEntry> &EntryStorage) { + if (TEntry) { + CachedFileContents *StoredContents = nullptr; + if (TEntry->Contents) + StoredContents = new (ContentsStorage.Allocate()) + CachedFileContents(std::move(TEntry->Contents)); + return new (EntryStorage.Allocate()) + CachedFileSystemEntry(std::move(TEntry->Status), StoredContents); + } else { + return new (EntryStorage.Allocate()) + CachedFileSystemEntry(TEntry.getError()); + } +} + const CachedFileSystemEntry * DependencyScanningWorkerFilesystem::resolveUIDThroughSharedCache( StringRef OriginalFilename, const llvm::vfs::Status &Stat) { @@ -245,8 +279,8 @@ DependencyScanningWorkerFilesystem::resolveUIDThroughSharedCache( } auto UIDProducer = std::move(std::get<SlotProducer>(UIDSlot)); - auto TEntry = - Stat.isDirectory() ? TentativeEntry(Stat) : readFile(OriginalFilename); + auto TEntry = Stat.isDirectory() ? TentativeEntry(Stat) + : readFile(OriginalFilename, *BaseFS); // Allocate the entry and bind the UID slot under one shard-lock acquisition // (BumpPtrAllocator isn't thread-safe). On read-failure, the entry wraps @@ -257,17 +291,8 @@ DependencyScanningWorkerFilesystem::resolveUIDThroughSharedCache( std::lock_guard<std::mutex> ShardLock(UIDShard.CacheLock); auto &State = UIDShard.EntriesByUID[Stat.getUniqueID()]; assert(!State.Entry && "UID slot already published an entry"); - if (TEntry) { - CachedFileContents *StoredContents = nullptr; - if (TEntry->Contents) - StoredContents = new (UIDShard.ContentsStorage.Allocate()) - CachedFileContents(std::move(TEntry->Contents)); - SharedEntry = new (UIDShard.EntryStorage.Allocate()) - CachedFileSystemEntry(std::move(TEntry->Status), StoredContents); - } else { - SharedEntry = new (UIDShard.EntryStorage.Allocate()) - CachedFileSystemEntry(TEntry.getError()); - } + SharedEntry = createSharedEntry(TEntry, UIDShard.ContentsStorage, + UIDShard.EntryStorage); State.Entry = SharedEntry; State.InProgress.reset(); } @@ -275,9 +300,23 @@ DependencyScanningWorkerFilesystem::resolveUIDThroughSharedCache( return SharedEntry; } +const CachedFileSystemEntry * +DependencyScanningWorkerFilesystem::createCachedFSEntryForLocal( + StringRef OriginalFilename, const llvm::vfs::Status &Stat) { + assert(LocalFS); + auto TEntry = Stat.isDirectory() ? TentativeEntry(Stat) + : readFile(OriginalFilename, *LocalFS); + return createSharedEntry(TEntry, LocalContentsStorage, LocalEntryStorage); +} + llvm::ErrorOr<const CachedFileSystemEntry *> DependencyScanningWorkerFilesystem::resolveFilenameThroughSharedCache( StringRef OriginalFilename, StringRef FilenameForLookup) { + if (LocalFS) { + if (auto Stat = LocalFS->status(OriginalFilename)) { + return createCachedFSEntryForLocal(OriginalFilename, *Stat); + } + } assert(llvm::sys::path::is_absolute_gnu(FilenameForLookup)); auto &FilenameShard = Service.getSharedCache().getShardForFilename(FilenameForLookup); @@ -295,7 +334,7 @@ DependencyScanningWorkerFilesystem::resolveFilenameThroughSharedCache( // - Stat failed, not cacheable: publish the error to current waiters but // don't persist; a later separate query re-runs the stat (so a file // created mid-scan becomes visible). - auto Stat = getUnderlyingFS().status(OriginalFilename); + auto Stat = BaseFS->status(OriginalFilename); const bool ShouldCacheNegativeStat = !Stat && Service.getOpts().CacheNegativeStats && shouldCacheNegativeStatsForPath(OriginalFilename); diff --git a/clang/lib/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/DependencyScanning/DependencyScanningWorker.cpp index 1196369174e9b4..af9946a64feba1 100644 --- a/clang/lib/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/DependencyScanning/DependencyScanningWorker.cpp @@ -995,7 +995,8 @@ class CompilerInstanceWithContext { } // namespace clang DependencyScanningWorker::DependencyScanningWorker( - DependencyScanningService &Service) + DependencyScanningService &Service, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> LocalFS) : Service(Service) { PCHContainerOps = std::make_shared<PCHContainerOperations>(); // We need to read object files from PCH built outside the scanner. @@ -1013,7 +1014,7 @@ DependencyScanningWorker::DependencyScanningWorker( } DepFS = llvm::makeIntrusiveRefCnt<DependencyScanningWorkerFilesystem>( - Service, std::move(BaseFS)); + Service, std::move(BaseFS), std::move(LocalFS)); } DependencyScanningWorker::~DependencyScanningWorker() = default; diff --git a/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp index e26744c75ee28c..9f1c966eb34d2d 100644 --- a/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp +++ b/clang/unittests/DependencyScanning/DependencyScanningFilesystemTest.cpp @@ -550,3 +550,63 @@ TEST(DependencyScanningWorkerFilesystem, EXPECT_FALSE(PostWorker.status(MissingPath)); EXPECT_EQ(TracingFS->NumStatusCalls.load(), StatsAfterBurst + 1); } + +TEST(DependencyScanningWorkerFilesystem, LocalFS) { + auto GlobalFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); + GlobalFS->setCurrentWorkingDirectory("/"); + GlobalFS->addFile("/glob.c", 0, llvm::MemoryBuffer::getMemBuffer("")); + + DependencyScanningServiceOptions Opts; + DependencyScanningService Service(std::move(Opts)); + + { + auto InstrGlobalFS = + llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(GlobalFS); + auto LocalFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); + LocalFS->setCurrentWorkingDirectory("/"); + LocalFS->addFile("/loc1.c", 0, llvm::MemoryBuffer::getMemBuffer("")); + auto InstrLocalFS = + llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(LocalFS); + + DependencyScanningWorkerFilesystem DepFS(Service, InstrGlobalFS, + InstrLocalFS); + + DepFS.status("/glob.c"); + EXPECT_EQ(InstrGlobalFS->NumStatusCalls, 1u); + + DepFS.status("/glob.c"); + EXPECT_EQ(InstrGlobalFS->NumStatusCalls, 1u); // Cached, no increase. + + // LocalFS got a stat call for "/glob.c". + EXPECT_EQ(InstrLocalFS->NumStatusCalls, 1u); + + auto Stat = DepFS.status("/loc1.c"); + ASSERT_TRUE(Stat); + EXPECT_EQ(InstrLocalFS->NumStatusCalls, 2u); + + DepFS.status("/loc1.c"); + EXPECT_EQ(InstrLocalFS->NumStatusCalls, 2u); // Cached, no increase. + } + + { + auto InstrGlobalFS = + llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(GlobalFS); + auto LocalFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); + LocalFS->setCurrentWorkingDirectory("/"); + auto InstrLocalFS = + llvm::makeIntrusiveRefCnt<llvm::vfs::TracingFileSystem>(LocalFS); + + DependencyScanningWorkerFilesystem DepFS(Service, InstrGlobalFS, + InstrLocalFS); + + DepFS.status("/glob.c"); + EXPECT_EQ(InstrGlobalFS->NumStatusCalls, 0u); // Cached, no increase. + + // LocalFS got a stat call for "/glob.c". + EXPECT_EQ(InstrLocalFS->NumStatusCalls, 1u); + + auto Stat = DepFS.status("/loc1.c"); + ASSERT_FALSE(Stat); + EXPECT_EQ(InstrLocalFS->NumStatusCalls, 2u); // not using global cache. + } +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
