Skip to content

Commit 39ed3c6

Browse files
authored
[clang-scan-deps] Fix contention when updating TrackingStatistics 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 #88152 (comment), 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.
1 parent 76a3be7 commit 39ed3c6

File tree

4 files changed

+25
-6
lines changed

4 files changed

+25
-6
lines changed

clang/include/clang/Basic/FileManager.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ class FileManager : public RefCountedBase<FileManager> {
114114
///
115115
unsigned NextFileUID;
116116

117+
/// Statistics gathered during the lifetime of the FileManager.
118+
unsigned NumDirLookups = 0;
119+
unsigned NumFileLookups = 0;
120+
unsigned NumDirCacheMisses = 0;
121+
unsigned NumFileCacheMisses = 0;
122+
117123
// Caching.
118124
std::unique_ptr<FileSystemStatCache> StatCache;
119125

@@ -341,6 +347,10 @@ class FileManager : public RefCountedBase<FileManager> {
341347

342348
public:
343349
void PrintStats() const;
350+
351+
/// Import statistics from a child FileManager and add them to this current
352+
/// FileManager.
353+
void AddStats(const FileManager &Other);
344354
};
345355

346356
} // end namespace clang

clang/lib/Basic/FileManager.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,6 @@ using namespace clang;
3939

4040
#define DEBUG_TYPE "file-search"
4141

42-
ALWAYS_ENABLED_STATISTIC(NumDirLookups, "Number of directory lookups.");
43-
ALWAYS_ENABLED_STATISTIC(NumFileLookups, "Number of file lookups.");
44-
ALWAYS_ENABLED_STATISTIC(NumDirCacheMisses,
45-
"Number of directory cache misses.");
46-
ALWAYS_ENABLED_STATISTIC(NumFileCacheMisses, "Number of file cache misses.");
47-
4842
//===----------------------------------------------------------------------===//
4943
// Common logic.
5044
//===----------------------------------------------------------------------===//
@@ -656,6 +650,14 @@ StringRef FileManager::getCanonicalName(const void *Entry, StringRef Name) {
656650
return CanonicalName;
657651
}
658652

653+
void FileManager::AddStats(const FileManager &Other) {
654+
assert(&Other != this && "Collecting stats into the same FileManager");
655+
NumDirLookups += Other.NumDirLookups;
656+
NumFileLookups += Other.NumFileLookups;
657+
NumDirCacheMisses += Other.NumDirCacheMisses;
658+
NumFileCacheMisses += Other.NumFileCacheMisses;
659+
}
660+
659661
void FileManager::PrintStats() const {
660662
llvm::errs() << "\n*** File Manager Stats:\n";
661663
llvm::errs() << UniqueRealFiles.size() << " real files found, "

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,10 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
12931293
diag::remark_module_build_done)
12941294
<< ModuleName;
12951295

1296+
// Propagate the statistics to the parent FileManager.
1297+
if (!FrontendOpts.ModulesShareFileManager)
1298+
ImportingInstance.getFileManager().AddStats(Instance.getFileManager());
1299+
12961300
if (Crashed) {
12971301
// Clear the ASTConsumer if it hasn't been already, in case it owns streams
12981302
// that must be closed before clearing output files.

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,9 @@ class DependencyScanningAction : public tooling::ToolAction {
439439
if (Result)
440440
setLastCC1Arguments(std::move(OriginalInvocation));
441441

442+
// Propagate the statistics to the parent FileManager.
443+
DriverFileMgr->AddStats(ScanInstance.getFileManager());
444+
442445
return Result;
443446
}
444447

0 commit comments

Comments
 (0)