[clang] 39ed3c6 - [clang-scan-deps] Fix contention when updating `TrackingStatistic`s in hot code paths in `FileManager`. (#88427)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 25 07:31:50 PDT 2024
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.
![Screenshot 2024-04-10
214123](https://github.com/llvm/llvm-project/assets/37383324/5756b1bc-cab5-4612-8769-ee7e03a66479)
![Screenshot 2024-04-10
214246](https://github.com/llvm/llvm-project/assets/37383324/3d560e89-61c7-4fb9-9330-f9e660e8fc8b)
![Screenshot 2024-04-10
214315](https://github.com/llvm/llvm-project/assets/37383324/006341fc-49d4-4720-a348-7af435c21b17)
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;
}
More information about the cfe-commits
mailing list