diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -26,11 +26,11 @@ /// the dependency scanning filesystem. /// /// It represents one of the following: -/// - an opened source file with minimized contents and a stat value. -/// - an opened source file with original contents and a stat value. -/// - a directory entry with its stat value. -/// - an error value to represent a file system error. -/// - a placeholder with an invalid stat indicating a not yet initialized entry. +/// - opened file with original contents and a stat value, +/// - opened file with original contents, minimized contents and a stat value, +/// - directory entry with its stat value, +/// - filesystem error, +/// - placeholder with an invalid stat indicating a not yet initialized entry. class CachedFileSystemEntry { public: /// Default constructor creates an entry with an invalid stat. @@ -38,32 +38,51 @@ CachedFileSystemEntry(std::error_code Error) : MaybeStat(std::move(Error)) {} - /// Create an entry that represents an opened source file with minimized or - /// original contents. + /// Create an entry that represents an opened source file with original + /// contents. /// /// The filesystem opens the file even for `stat` calls open to avoid the /// issues with stat + open of minimized files that might lead to a /// mismatching size of the file. static CachedFileSystemEntry createFileEntry(StringRef Filename, - llvm::vfs::FileSystem &FS, - bool Minimize = true); + llvm::vfs::FileSystem &FS); /// Create an entry that represents a directory on the filesystem. static CachedFileSystemEntry createDirectoryEntry(llvm::vfs::Status &&Stat); + /// Check whether the file still needs to be minimized. + bool needsMinimization(bool ShouldBeMinimized) const { + return ShouldBeMinimized && MaybeStat && !MaybeStat->isDirectory() && + MinimizedContents == nullptr; + } + + /// Minimize contents of the file. + void minimize(); + /// \returns True if the entry is valid. bool isValid() const { return !MaybeStat || MaybeStat->isStatusKnown(); } /// \returns True if the current entry points to a directory. bool isDirectory() const { return MaybeStat && MaybeStat->isDirectory(); } - /// \returns The error or the file's contents. - llvm::ErrorOr getContents() const { + /// \returns The error or the file's original contents. + llvm::ErrorOr getOriginalContents() const { if (!MaybeStat) return MaybeStat.getError(); assert(!MaybeStat->isDirectory() && "not a file"); assert(isValid() && "not initialized"); - return Contents->getBuffer(); + assert(OriginalContents && "not read"); + return OriginalContents->getBuffer(); + } + + /// \returns The error or the file's minimized contents. + llvm::ErrorOr getMinimizedContents() const { + if (!MaybeStat) + return MaybeStat.getError(); + assert(!MaybeStat->isDirectory() && "not a file"); + assert(isValid() && "not initialized"); + assert(MinimizedContents && "not minimized"); + return MinimizedContents->getBuffer(); } /// \returns The error or the status of the entry. @@ -92,7 +111,8 @@ private: llvm::ErrorOr MaybeStat; - std::unique_ptr Contents; + std::unique_ptr OriginalContents; + std::unique_ptr MinimizedContents; PreprocessorSkippedRangeMapping PPSkippedRangeMapping; }; @@ -109,61 +129,69 @@ CachedFileSystemEntry Value; }; + DependencyScanningFilesystemSharedCache(); + /// Returns a cache entry for the corresponding key. /// /// A new cache entry is created if the key is not in the cache. This is a /// thread safe call. - SharedFileSystemEntry &get(StringRef Key, bool Minimized); + SharedFileSystemEntry &get(StringRef Key); private: - class SingleCache { - public: - SingleCache(); - - SharedFileSystemEntry &get(StringRef Key); - - private: - struct CacheShard { - std::mutex CacheLock; - llvm::StringMap Cache; - }; - std::unique_ptr CacheShards; - unsigned NumShards; + struct CacheShard { + std::mutex CacheLock; + llvm::StringMap Cache; }; - - SingleCache CacheMinimized; - SingleCache CacheOriginal; + std::unique_ptr CacheShards; + unsigned NumShards; }; /// This class is a local cache, that caches the 'stat' and 'open' calls to the /// underlying real file system. It distinguishes between minimized and original /// files. class DependencyScanningFilesystemLocalCache { -private: - using SingleCache = - llvm::StringMap; + llvm::StringMap Cache; - SingleCache CacheMinimized; - SingleCache CacheOriginal; - - SingleCache &selectCache(bool Minimized) { - return Minimized ? CacheMinimized : CacheOriginal; +public: + const CachedFileSystemEntry *getCachedEntry(StringRef Filename) { + return Cache[Filename]; } +}; + +/// Reference to a CachedFileSystemEntry. +/// If the underlying entry is an open file, this wrapper returns the correct +/// contents (original or minimized) and ensures consistency with file size +/// reported by status. +class EntryRef { + /// For entry that is an open files, this signifies whether it is minimized. + bool Minimized; + + /// The underlying cached entry. + const CachedFileSystemEntry *Entry; public: - void setCachedEntry(StringRef Filename, bool Minimized, - const CachedFileSystemEntry *Entry) { - SingleCache &Cache = selectCache(Minimized); - bool IsInserted = Cache.try_emplace(Filename, Entry).second; - (void)IsInserted; - assert(IsInserted && "local cache is updated more than once"); + EntryRef(bool Minimized, const CachedFileSystemEntry *Entry) + : Minimized(Minimized), Entry(Entry) {} + + llvm::ErrorOr getStatus() const { + auto MaybeStat = Entry->getStatus(); + if (!MaybeStat || MaybeStat->isDirectory()) + return MaybeStat; + return llvm::vfs::Status::copyWithNewSize(*MaybeStat, + getContents()->size()); } - const CachedFileSystemEntry *getCachedEntry(StringRef Filename, - bool Minimized) { - SingleCache &Cache = selectCache(Minimized); - auto It = Cache.find(Filename); - return It == Cache.end() ? nullptr : It->getValue(); + bool isDirectory() const { return Entry->isDirectory(); } + + StringRef getName() const { return Entry->getName(); } + + llvm::ErrorOr getContents() const { + return Minimized ? Entry->getMinimizedContents() + : Entry->getOriginalContents(); + } + + const PreprocessorSkippedRangeMapping &getPPSkippedRangeMapping() const { + return Entry->getPPSkippedRangeMapping(); } }; @@ -198,13 +226,13 @@ /// Check whether the file should be minimized. bool shouldMinimize(StringRef Filename); - llvm::ErrorOr - getOrCreateFileSystemEntry(const StringRef Filename); + llvm::ErrorOr getOrCreateFileSystemEntry(const StringRef Filename, + bool ShouldBeMinimized); /// Create a cached file system entry based on the initial status result. CachedFileSystemEntry createFileSystemEntry(llvm::ErrorOr &&MaybeStatus, - StringRef Filename, bool ShouldMinimize); + StringRef Filename); /// The global cache shared between worker threads. DependencyScanningFilesystemSharedCache &SharedCache; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -16,8 +16,9 @@ using namespace tooling; using namespace dependencies; -CachedFileSystemEntry CachedFileSystemEntry::createFileEntry( - StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize) { +CachedFileSystemEntry +CachedFileSystemEntry::createFileEntry(StringRef Filename, + llvm::vfs::FileSystem &FS) { // Load the file and its content from the file system. llvm::ErrorOr> MaybeFile = FS.openFileForRead(Filename); @@ -33,31 +34,28 @@ if (!MaybeBuffer) return MaybeBuffer.getError(); + CachedFileSystemEntry Result; + Result.MaybeStat = std::move(*Stat); + Result.OriginalContents = std::move(*MaybeBuffer); + return Result; +} + +void CachedFileSystemEntry::minimize() { llvm::SmallString<1024> MinimizedFileContents; // Minimize the file down to directives that might affect the dependencies. - const auto &Buffer = *MaybeBuffer; SmallVector Tokens; - if (!Minimize || minimizeSourceToDependencyDirectives( - Buffer->getBuffer(), MinimizedFileContents, Tokens)) { - // Use the original file unless requested otherwise, or - // if the minimization failed. - // FIXME: Propage the diagnostic if desired by the client. - CachedFileSystemEntry Result; - Result.MaybeStat = std::move(*Stat); - Result.Contents = std::move(*MaybeBuffer); - return Result; + if (minimizeSourceToDependencyDirectives( + OriginalContents->getBuffer(), MinimizedFileContents, Tokens)) { + // FIXME: Propagate the diagnostic if desired by the client. + // Use the original file if the minimization failed. + MinimizedContents = llvm::MemoryBuffer::getMemBuffer(*OriginalContents); + return; } - CachedFileSystemEntry Result; - size_t Size = MinimizedFileContents.size(); - Result.MaybeStat = llvm::vfs::Status(Stat->getName(), Stat->getUniqueID(), - Stat->getLastModificationTime(), - Stat->getUser(), Stat->getGroup(), Size, - Stat->getType(), Stat->getPermissions()); // The contents produced by the minimizer must be null terminated. assert(MinimizedFileContents.data()[MinimizedFileContents.size()] == '\0' && "not null terminated contents"); - Result.Contents = std::make_unique( + MinimizedContents = std::make_unique( std::move(MinimizedFileContents)); // Compute the skipped PP ranges that speedup skipping over inactive @@ -76,9 +74,7 @@ } Mapping[Range.Offset] = Range.Length; } - Result.PPSkippedRangeMapping = std::move(Mapping); - - return Result; + PPSkippedRangeMapping = std::move(Mapping); } CachedFileSystemEntry @@ -89,7 +85,8 @@ return Result; } -DependencyScanningFilesystemSharedCache::SingleCache::SingleCache() { +DependencyScanningFilesystemSharedCache:: + DependencyScanningFilesystemSharedCache() { // This heuristic was chosen using a empirical testing on a // reasonably high core machine (iMacPro 18 cores / 36 threads). The cache // sharding gives a performance edge by reducing the lock contention. @@ -101,19 +98,13 @@ } DependencyScanningFilesystemSharedCache::SharedFileSystemEntry & -DependencyScanningFilesystemSharedCache::SingleCache::get(StringRef Key) { +DependencyScanningFilesystemSharedCache::get(StringRef Key) { CacheShard &Shard = CacheShards[llvm::hash_value(Key) % NumShards]; std::lock_guard LockGuard(Shard.CacheLock); auto It = Shard.Cache.try_emplace(Key); return It.first->getValue(); } -DependencyScanningFilesystemSharedCache::SharedFileSystemEntry & -DependencyScanningFilesystemSharedCache::get(StringRef Key, bool Minimized) { - SingleCache &Cache = Minimized ? CacheMinimized : CacheOriginal; - return Cache.get(Key); -} - /// Whitelist file extensions that should be minimized, treating no extension as /// a source file that should be minimized. /// @@ -158,32 +149,27 @@ } CachedFileSystemEntry DependencyScanningWorkerFilesystem::createFileSystemEntry( - llvm::ErrorOr &&MaybeStatus, StringRef Filename, - bool ShouldMinimize) { + llvm::ErrorOr &&MaybeStatus, StringRef Filename) { if (!MaybeStatus) return CachedFileSystemEntry(MaybeStatus.getError()); if (MaybeStatus->isDirectory()) return CachedFileSystemEntry::createDirectoryEntry(std::move(*MaybeStatus)); - return CachedFileSystemEntry::createFileEntry(Filename, getUnderlyingFS(), - ShouldMinimize); + return CachedFileSystemEntry::createFileEntry(Filename, getUnderlyingFS()); } -llvm::ErrorOr +llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( - const StringRef Filename) { - bool ShouldMinimize = shouldMinimize(Filename); - - if (const auto *Entry = Cache.getCachedEntry(Filename, ShouldMinimize)) - return Entry; + const StringRef Filename, bool ShouldBeMinimized) { + const auto *Entry = Cache.getCachedEntry(Filename); + if (Entry && !Entry->needsMinimization(ShouldBeMinimized)) + return EntryRef(ShouldBeMinimized, Entry); // FIXME: Handle PCM/PCH files. // FIXME: Handle module map files. - DependencyScanningFilesystemSharedCache::SharedFileSystemEntry - &SharedCacheEntry = SharedCache.get(Filename, ShouldMinimize); - const CachedFileSystemEntry *Result; + auto &SharedCacheEntry = SharedCache.get(Filename); { std::lock_guard LockGuard(SharedCacheEntry.ValueLock); CachedFileSystemEntry &CacheEntry = SharedCacheEntry.Value; @@ -196,27 +182,28 @@ // files before building them, and then looks for them again. If we // cache the stat failure, it won't see them the second time. return MaybeStatus.getError(); - CacheEntry = createFileSystemEntry(std::move(MaybeStatus), Filename, - ShouldMinimize); + CacheEntry = createFileSystemEntry(std::move(MaybeStatus), Filename); } - Result = &CacheEntry; + if (CacheEntry.needsMinimization(ShouldBeMinimized)) + CacheEntry.minimize(); } // Store the result in the local cache. - Cache.setCachedEntry(Filename, ShouldMinimize, Result); - return Result; + Entry = &SharedCacheEntry.Value; + return EntryRef(ShouldBeMinimized, Entry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::status(const Twine &Path) { SmallString<256> OwnedFilename; StringRef Filename = Path.toStringRef(OwnedFilename); - const llvm::ErrorOr Result = - getOrCreateFileSystemEntry(Filename); + bool ShouldBeMinimized = shouldMinimize(Filename); + llvm::ErrorOr Result = + getOrCreateFileSystemEntry(Filename, ShouldBeMinimized); if (!Result) return Result.getError(); - return (*Result)->getStatus(); + return Result->getStatus(); } namespace { @@ -230,7 +217,7 @@ : Buffer(std::move(Buffer)), Stat(std::move(Stat)) {} static llvm::ErrorOr> - create(const CachedFileSystemEntry *Entry, + create(EntryRef Entry, ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings); llvm::ErrorOr status() override { return Stat; } @@ -251,21 +238,20 @@ } // end anonymous namespace llvm::ErrorOr> MinimizedVFSFile::create( - const CachedFileSystemEntry *Entry, - ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) { - if (Entry->isDirectory()) + EntryRef Entry, ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) { + if (Entry.isDirectory()) return llvm::ErrorOr>( std::make_error_code(std::errc::is_a_directory)); - llvm::ErrorOr Contents = Entry->getContents(); + llvm::ErrorOr Contents = Entry.getContents(); if (!Contents) return Contents.getError(); auto Result = std::make_unique( - llvm::MemoryBuffer::getMemBuffer(*Contents, Entry->getName(), + llvm::MemoryBuffer::getMemBuffer(*Contents, Entry.getName(), /*RequiresNullTerminator=*/false), - *Entry->getStatus()); - if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings) + *Entry.getStatus()); + if (!Entry.getPPSkippedRangeMapping().empty() && PPSkipMappings) (*PPSkipMappings)[Result->Buffer->getBufferStart()] = - &Entry->getPPSkippedRangeMapping(); + &Entry.getPPSkippedRangeMapping(); return llvm::ErrorOr>( std::unique_ptr(std::move(Result))); } @@ -275,8 +261,9 @@ SmallString<256> OwnedFilename; StringRef Filename = Path.toStringRef(OwnedFilename); - const llvm::ErrorOr Result = - getOrCreateFileSystemEntry(Filename); + bool ShouldBeMinimized = shouldMinimize(Filename); + llvm::ErrorOr Result = + getOrCreateFileSystemEntry(Filename, ShouldBeMinimized); if (!Result) return Result.getError(); return MinimizedVFSFile::create(Result.get(), PPSkipMappings); diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -205,32 +205,46 @@ } namespace dependencies { -TEST(DependencyScanningFilesystem, IgnoredFilesHaveSeparateCache) { +TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately1) { auto VFS = llvm::makeIntrusiveRefCnt(); - VFS->addFile("/mod.h", 0, llvm::MemoryBuffer::getMemBuffer("// hi there!\n")); + VFS->addFile("/mod.h", 0, + llvm::MemoryBuffer::getMemBuffer("#include \n" + "// hi there!\n")); DependencyScanningFilesystemSharedCache SharedCache; auto Mappings = std::make_unique(); DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get()); + DepFS.enableMinimizationOfAllFiles(); // Let's be explicit for clarity. auto StatusMinimized0 = DepFS.status("/mod.h"); DepFS.disableMinimization("/mod.h"); auto StatusFull1 = DepFS.status("/mod.h"); - DepFS.enableMinimizationOfAllFiles(); - - auto StatusMinimized2 = DepFS.status("/mod.h"); - DepFS.disableMinimization("/mod.h"); - auto StatusFull3 = DepFS.status("/mod.h"); EXPECT_TRUE(StatusMinimized0); - EXPECT_EQ(StatusMinimized0->getSize(), 0u); EXPECT_TRUE(StatusFull1); - EXPECT_EQ(StatusFull1->getSize(), 13u); + EXPECT_EQ(StatusMinimized0->getSize(), 17u); + EXPECT_EQ(StatusFull1->getSize(), 30u); +} + +TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) { + auto VFS = llvm::makeIntrusiveRefCnt(); + VFS->addFile("/mod.h", 0, + llvm::MemoryBuffer::getMemBuffer("#include \n" + "// hi there!\n")); + + DependencyScanningFilesystemSharedCache SharedCache; + auto Mappings = std::make_unique(); + DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get()); + + DepFS.disableMinimization("/mod.h"); + auto StatusFull0 = DepFS.status("/mod.h"); + DepFS.enableMinimizationOfAllFiles(); + auto StatusMinimized1 = DepFS.status("/mod.h"); - EXPECT_TRUE(StatusMinimized2); - EXPECT_EQ(StatusMinimized2->getSize(), 0u); - EXPECT_TRUE(StatusFull3); - EXPECT_EQ(StatusFull3->getSize(), 13u); + EXPECT_TRUE(StatusFull0); + EXPECT_TRUE(StatusMinimized1); + EXPECT_EQ(StatusFull0->getSize(), 30u); + EXPECT_EQ(StatusMinimized1->getSize(), 17u); } } // end namespace dependencies diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -64,6 +64,8 @@ uint64_t Size, llvm::sys::fs::file_type Type, llvm::sys::fs::perms Perms); + /// Get a copy of a Status with a different size. + static Status copyWithNewSize(const Status &In, uint64_t NewSize); /// Get a copy of a Status with a different name. static Status copyWithNewName(const Status &In, const Twine &NewName); static Status copyWithNewName(const llvm::sys::fs::file_status &In, diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -75,6 +75,12 @@ : Name(Name.str()), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size), Type(Type), Perms(Perms) {} +Status Status::copyWithNewSize(const Status &In, uint64_t NewSize) { + return Status(In.getName(), In.getUniqueID(), In.getLastModificationTime(), + In.getUser(), In.getGroup(), NewSize, In.getType(), + In.getPermissions()); +} + Status Status::copyWithNewName(const Status &In, const Twine &NewName) { return Status(NewName, In.getUniqueID(), In.getLastModificationTime(), In.getUser(), In.getGroup(), In.getSize(), In.getType(),