Author: Alexandre Ganea Date: 2024-04-25T10:31:45-04:00 New Revision: 39ed3c68e51f1b04fe2890db9006ae1b176b1582
URL: https://github.com/llvm/llvm-project/commit/39ed3c68e51f1b04fe2890db9006ae1b176b1582 DIFF: https://github.com/llvm/llvm-project/commit/39ed3c68e51f1b04fe2890db9006ae1b176b1582.diff LOG: [clang-scan-deps] Fix contention when updating `TrackingStatistic`s in hot code paths in `FileManager`. (#88427) `FileManager::getDirectoryRef()` and `FileManager::getFileRef()` are hot code paths in `clang-scan-deps`. These functions are updating on every call a few atomics related to printing statistics, which causes contention on high core count machines.    After this patch we make the variables local to the `FileManager`. In our test case, this saves about 49 sec over 1 min 47 sec of `clang-scan-deps` run time (1 min 47 sec before, 58 sec after). These figures are after applying my suggestion in https://github.com/llvm/llvm-project/pull/88152#issuecomment-2049803229, that is: ``` static bool shouldCacheStatFailures(StringRef Filename) { return true; } ``` Without the above, there's just too much OS noise from the high volume of `status()` calls with regular non-modules C++ code. Tested on Windows with clang-cl. Added: Modified: clang/include/clang/Basic/FileManager.h clang/lib/Basic/FileManager.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 2245fd78bfc9f0..8b4206e52cd482 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -114,6 +114,12 @@ class FileManager : public RefCountedBase<FileManager> { /// unsigned NextFileUID; + /// Statistics gathered during the lifetime of the FileManager. + unsigned NumDirLookups = 0; + unsigned NumFileLookups = 0; + unsigned NumDirCacheMisses = 0; + unsigned NumFileCacheMisses = 0; + // Caching. std::unique_ptr<FileSystemStatCache> StatCache; @@ -341,6 +347,10 @@ class FileManager : public RefCountedBase<FileManager> { public: void PrintStats() const; + + /// Import statistics from a child FileManager and add them to this current + /// FileManager. + void AddStats(const FileManager &Other); }; } // end namespace clang diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index cd520a6375e07e..143c04309d0753 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -39,12 +39,6 @@ using namespace clang; #define DEBUG_TYPE "file-search" -ALWAYS_ENABLED_STATISTIC(NumDirLookups, "Number of directory lookups."); -ALWAYS_ENABLED_STATISTIC(NumFileLookups, "Number of file lookups."); -ALWAYS_ENABLED_STATISTIC(NumDirCacheMisses, - "Number of directory cache misses."); -ALWAYS_ENABLED_STATISTIC(NumFileCacheMisses, "Number of file cache misses."); - //===----------------------------------------------------------------------===// // Common logic. //===----------------------------------------------------------------------===// @@ -656,6 +650,14 @@ StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) { return CanonicalName; } +void FileManager::AddStats(const FileManager &Other) { + assert(&Other != this && "Collecting stats into the same FileManager"); + NumDirLookups += Other.NumDirLookups; + NumFileLookups += Other.NumFileLookups; + NumDirCacheMisses += Other.NumDirCacheMisses; + NumFileCacheMisses += Other.NumFileCacheMisses; +} + void FileManager::PrintStats() const { llvm::errs() << "\n*** File Manager Stats:\n"; llvm::errs() << UniqueRealFiles.size() << " real files found, " diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 6e3baf83864415..66a45b888f15cc 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1293,6 +1293,10 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, diag::remark_module_build_done) << ModuleName; + // Propagate the statistics to the parent FileManager. + if (!FrontendOpts.ModulesShareFileManager) + ImportingInstance.getFileManager().AddStats(Instance.getFileManager()); + if (Crashed) { // Clear the ASTConsumer if it hasn't been already, in case it owns streams // that must be closed before clearing output files. diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 32850f5eea92a9..0c047b6c5da2f8 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -439,6 +439,9 @@ class DependencyScanningAction : public tooling::ToolAction { if (Result) setLastCC1Arguments(std::move(OriginalInvocation)); + // Propagate the statistics to the parent FileManager. + DriverFileMgr->AddStats(ScanInstance.getFileManager()); + return Result; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
