[clang] [clang-scan-deps] Fix contention when updating `TrackingStatistic`s in hot code paths in `FileManager`. (PR #88427)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 11 12:31:32 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Alexandre Ganea (aganea)
<details>
<summary>Changes</summary>
`FileManager::getDirectoryRef` and `FileManager::getFileRef` are hot code paths in `clang-scan-deps`. In these functions, we update a couple of atomic variables related to printing statistics, which causes contention on high core count machines.
After this PR, we update these variables iff the user wants to print statistics.
On my use 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 that, there's just too much OS noise from the high volume of `status()` calls with regular non-module C++ code. Tested on Windows with clang-cl.
---
Full diff: https://github.com/llvm/llvm-project/pull/88427.diff
8 Files Affected:
- (modified) clang/include/clang/Basic/FileManager.h (+5-1)
- (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h (+5-1)
- (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h (+2)
- (modified) clang/lib/Basic/FileManager.cpp (+15-8)
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+2-1)
- (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp (+2-2)
- (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+4-2)
- (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+1-1)
``````````diff
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 2245fd78bfc9f0..24256a7368ccc8 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -114,6 +114,9 @@ class FileManager : public RefCountedBase<FileManager> {
///
unsigned NextFileUID;
+ /// Whether we want to print statistics. This impacts the collection of data.
+ bool EnablePrintStats;
+
// Caching.
std::unique_ptr<FileSystemStatCache> StatCache;
@@ -134,7 +137,8 @@ class FileManager : public RefCountedBase<FileManager> {
/// \param FS if non-null, the VFS to use. Otherwise uses
/// llvm::vfs::getRealFileSystem().
FileManager(const FileSystemOptions &FileSystemOpts,
- IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr,
+ bool PrintStats = false);
~FileManager();
/// Installs the provided FileSystemStatCache object within
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 557f0e547ab4a8..7b869bb7976f2a 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -76,7 +76,7 @@ class DependencyScanningService {
DependencyScanningService(
ScanningMode Mode, ScanningOutputFormat Format,
ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
- bool EagerLoadModules = false);
+ bool EagerLoadModules = false, bool PrintStats = false);
ScanningMode getMode() const { return Mode; }
@@ -90,6 +90,8 @@ class DependencyScanningService {
return SharedCache;
}
+ bool getPrintStats() const { return PrintStats; }
+
private:
const ScanningMode Mode;
const ScanningOutputFormat Format;
@@ -97,6 +99,8 @@ class DependencyScanningService {
const ScanningOptimizations OptimizeArgs;
/// Whether to set up command-lines to load PCM files eagerly.
const bool EagerLoadModules;
+ /// Whether we should collect statistics during execution.
+ const bool PrintStats;
/// The global file system cache.
DependencyScanningFilesystemSharedCache SharedCache;
};
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 0f607862194b31..27b96c964ce83d 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -119,6 +119,8 @@ class DependencyScanningWorker {
ScanningOptimizations OptimizeArgs;
/// Whether to set up command-lines to load PCM files eagerly.
bool EagerLoadModules;
+ /// Whether we should collect statistics during execution.
+ bool PrintStats;
};
} // end namespace dependencies
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index cd520a6375e07e..1071f6ae53dd78 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -50,9 +50,10 @@ ALWAYS_ENABLED_STATISTIC(NumFileCacheMisses, "Number of file cache misses.");
//===----------------------------------------------------------------------===//
FileManager::FileManager(const FileSystemOptions &FSO,
- IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
+ bool PrintStats)
: FS(std::move(FS)), FileSystemOpts(FSO), SeenDirEntries(64),
- SeenFileEntries(64), NextFileUID(0) {
+ SeenFileEntries(64), NextFileUID(0), EnablePrintStats(PrintStats) {
// If the caller doesn't provide a virtual file system, just grab the real
// file system.
if (!this->FS)
@@ -134,7 +135,8 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
}
}
- ++NumDirLookups;
+ if (EnablePrintStats)
+ ++NumDirLookups;
// See if there was already an entry in the map. Note that the map
// contains both virtual and real directories.
@@ -147,7 +149,8 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
}
// We've not seen this before. Fill it in.
- ++NumDirCacheMisses;
+ if (EnablePrintStats)
+ ++NumDirCacheMisses;
auto &NamedDirEnt = *SeenDirInsertResult.first;
assert(!NamedDirEnt.second && "should be newly-created");
@@ -202,7 +205,8 @@ FileManager::getFile(StringRef Filename, bool openFile, bool CacheFailure) {
llvm::Expected<FileEntryRef>
FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
- ++NumFileLookups;
+ if (EnablePrintStats)
+ ++NumFileLookups;
// See if there is already an entry in the map.
auto SeenFileInsertResult =
@@ -215,7 +219,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
}
// We've not seen this before. Fill it in.
- ++NumFileCacheMisses;
+ if (EnablePrintStats)
+ ++NumFileCacheMisses;
auto *NamedFileEnt = &*SeenFileInsertResult.first;
assert(!NamedFileEnt->second && "should be newly-created");
@@ -377,7 +382,8 @@ const FileEntry *FileManager::getVirtualFile(StringRef Filename, off_t Size,
FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
time_t ModificationTime) {
- ++NumFileLookups;
+ if (EnablePrintStats)
+ ++NumFileLookups;
// See if there is already an entry in the map for an existing file.
auto &NamedFileEnt = *SeenFileEntries.insert(
@@ -390,7 +396,8 @@ FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size,
}
// We've not seen this before, or the file is cached as non-existent.
- ++NumFileCacheMisses;
+ if (EnablePrintStats)
+ ++NumFileCacheMisses;
addAncestorsAsVirtualDirs(Filename);
FileEntry *UFE = nullptr;
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 6e3baf83864415..d29bc92a953d66 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -381,7 +381,8 @@ FileManager *CompilerInstance::createFileManager(
: createVFSFromCompilerInvocation(getInvocation(),
getDiagnostics());
assert(VFS && "FileManager has no VFS?");
- FileMgr = new FileManager(getFileSystemOpts(), std::move(VFS));
+ FileMgr = new FileManager(getFileSystemOpts(), std::move(VFS),
+ getFrontendOpts().ShowStats);
return FileMgr.get();
}
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
index 7458ef484b16c4..584066df85aa66 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -15,9 +15,9 @@ using namespace dependencies;
DependencyScanningService::DependencyScanningService(
ScanningMode Mode, ScanningOutputFormat Format,
- ScanningOptimizations OptimizeArgs, bool EagerLoadModules)
+ ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool PrintStats)
: Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
- EagerLoadModules(EagerLoadModules) {
+ EagerLoadModules(EagerLoadModules), PrintStats(PrintStats) {
// Initialize targets for object file support.
llvm::InitializeAllTargets();
llvm::InitializeAllTargetMCs();
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 32850f5eea92a9..a684ba9cf5ca0e 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -501,6 +501,8 @@ DependencyScanningWorker::DependencyScanningWorker(
BaseFS = FS;
break;
}
+
+ PrintStats = Service.getPrintStats();
}
llvm::Error DependencyScanningWorker::computeDependencies(
@@ -623,8 +625,8 @@ bool DependencyScanningWorker::computeDependencies(
ModifiedCommandLine ? *ModifiedCommandLine : CommandLine;
auto &FinalFS = ModifiedFS ? ModifiedFS : BaseFS;
- auto FileMgr =
- llvm::makeIntrusiveRefCnt<FileManager>(FileSystemOptions{}, FinalFS);
+ auto FileMgr = llvm::makeIntrusiveRefCnt<FileManager>(FileSystemOptions{},
+ FinalFS, PrintStats);
std::vector<const char *> FinalCCommandLine(FinalCommandLine.size(), nullptr);
llvm::transform(FinalCommandLine, FinalCCommandLine.begin(),
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index eaa76dd43e41dd..bef96213cd6e54 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -973,7 +973,7 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
};
DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
- EagerLoadModules);
+ EagerLoadModules, PrintTiming);
llvm::Timer T;
T.startTimer();
``````````
</details>
https://github.com/llvm/llvm-project/pull/88427
More information about the cfe-commits
mailing list