[clang-tools-extra] [clang-tidy][NFC] optimize cache for config option (PR #121406)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 31 09:15:07 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Congcong Cai (HerrCai0907)
<details>
<summary>Changes</summary>
Current implement will cache `OptionsSource` for each path, it will create lots of copy of `OptionsSource` when project has deep nested folder structure.
New implement use vector to store `OptionsSource` and only cache the index. It can reduce memory usage and avoid meaningless copy.
---
Full diff: https://github.com/llvm/llvm-project/pull/121406.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.cpp (+24-23)
- (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.h (+4-1)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 445c7f85c900c6..9ecb255d916e9b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -337,33 +337,34 @@ FileOptionsBaseProvider::FileOptionsBaseProvider(
void FileOptionsBaseProvider::addRawFileOptions(
llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
auto CurSize = CurOptions.size();
-
// Look for a suitable configuration file in all parent directories of the
// file. Start with the immediate parent directory and move up.
- StringRef Path = llvm::sys::path::parent_path(AbsolutePath);
- for (StringRef CurrentPath = Path; !CurrentPath.empty();
- CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
- std::optional<OptionsSource> Result;
-
- auto Iter = CachedOptions.find(CurrentPath);
- if (Iter != CachedOptions.end())
- Result = Iter->second;
-
- if (!Result)
- Result = tryReadConfigFile(CurrentPath);
-
- if (Result) {
- // Store cached value for all intermediate directories.
- while (Path != CurrentPath) {
+ StringRef RootPath = llvm::sys::path::parent_path(AbsolutePath);
+ auto MemorizedConfigFile =
+ [this, &RootPath](StringRef CurrentPath) -> std::optional<OptionsSource> {
+ auto Iter = CachedOptions.Memorized.find(CurrentPath);
+ if (Iter != CachedOptions.Memorized.end())
+ return CachedOptions.Storage[Iter->second];
+ std::optional<OptionsSource> OptionsSource = tryReadConfigFile(CurrentPath);
+ if (OptionsSource) {
+ const size_t Index = CachedOptions.Storage.size();
+ CachedOptions.Storage.emplace_back(OptionsSource.value());
+ while (RootPath != CurrentPath) {
LLVM_DEBUG(llvm::dbgs()
- << "Caching configuration for path " << Path << ".\n");
- if (!CachedOptions.count(Path))
- CachedOptions[Path] = *Result;
- Path = llvm::sys::path::parent_path(Path);
+ << "Caching configuration for path " << RootPath << ".\n");
+ CachedOptions.Memorized[RootPath] = Index;
+ RootPath = llvm::sys::path::parent_path(RootPath);
}
- CachedOptions[Path] = *Result;
-
- CurOptions.push_back(*Result);
+ CachedOptions.Memorized[CurrentPath] = Index;
+ RootPath = llvm::sys::path::parent_path(CurrentPath);
+ }
+ return OptionsSource;
+ };
+ for (StringRef CurrentPath = RootPath; !CurrentPath.empty();
+ CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
+ if (std::optional<OptionsSource> Result =
+ MemorizedConfigFile(CurrentPath)) {
+ CurOptions.emplace_back(Result.value());
if (!Result->first.InheritParentConfig.value_or(false))
break;
}
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 85d5a02ebbc1bc..568f60cf98b21f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -241,7 +241,10 @@ class FileOptionsBaseProvider : public DefaultOptionsProvider {
/// \c ConfigHandlers.
std::optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
- llvm::StringMap<OptionsSource> CachedOptions;
+ struct OptionsCache {
+ llvm::StringMap<size_t> Memorized;
+ llvm::SmallVector<OptionsSource, 4U> Storage;
+ } CachedOptions;
ClangTidyOptions OverrideOptions;
ConfigFileHandlers ConfigHandlers;
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
``````````
</details>
https://github.com/llvm/llvm-project/pull/121406
More information about the cfe-commits
mailing list