[clang] [clang] Add a one-entry cache to DiagStateMap::getFile() (PR #228956)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Nico Weber (nico)

<details>
<summary>Changes</summary>

Once a TU has seen a `#pragma clang diagnostic` (libc++ has them in most headers), every DiagnosticsEngine::isIgnored() / getDiagnosticSeverity() call with a location looks up the location's FileID in a std::map<FileID, File>. The map has an entry for every FileID a state was ever looked up for, including macro expansions, so it gets large. For blink's logical_box_fragment.cc, it has 60755 entries, and 92.4% of the 3.28M lookups are for the same FileID as the previous lookup.

std::map has stable addresses, so remember the result of the last lookup.

For 60 random Chromium TUs (linux x64, -O2) picked with probability proportional to their compile time, sum over all TUs:

    CPU time: 192.6 s => 191.4 s, -0.65%
    instructions: 1903.0e9 => 1888.3e9, -0.77%

(To check noise level, running this with the same binary twice: -0.37% CPU time, +0.01% instructions)

No behavior change.

---
Full diff: https://github.com/llvm/llvm-project/pull/228956.diff


2 Files Affected:

- (modified) clang/include/clang/Basic/Diagnostic.h (+7) 
- (modified) clang/lib/Basic/Diagnostic.cpp (+11) 


``````````diff
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 72b1414638203cc..39b88c0624dfbef 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -437,6 +437,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
     void clear(bool Soft) {
       // Just clear the cache when in soft mode.
       Files.clear();
+      LastLookupFile = nullptr;
       if (!Soft) {
         FirstDiagState = CurDiagState = nullptr;
         CurDiagStateLoc = SourceLocation();
@@ -495,6 +496,11 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
     /// The diagnostic states for each file.
     mutable std::map<FileID, File> Files;
 
+    /// One-entry cache for getFile(): Files gets large, and most
+    /// lookups are for the same FileID as the previous one.
+    mutable FileID LastLookupFileID;
+    mutable File *LastLookupFile = nullptr;
+
     /// The initial diagnostic state.
     DiagState *FirstDiagState;
 
@@ -506,6 +512,7 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
 
     /// Get the diagnostic state information for a file.
     File *getFile(SourceManager &SrcMgr, FileID ID) const;
+    File *getFileUncached(SourceManager &SrcMgr, FileID ID) const;
   };
 
   DiagStateMap DiagStatesByLoc;
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 48dd9559ab8e631..5a05a3ff647465b 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -218,6 +218,17 @@ DiagnosticsEngine::DiagStateMap::File::lookup(unsigned Offset) const {
 DiagnosticsEngine::DiagStateMap::File *
 DiagnosticsEngine::DiagStateMap::getFile(SourceManager &SrcMgr,
                                          FileID ID) const {
+  if (!LastLookupFile || LastLookupFileID != ID) {
+    // getFileUncached() can recurse into getFile(), so update the cache after.
+    LastLookupFile = getFileUncached(SrcMgr, ID);
+    LastLookupFileID = ID;
+  }
+  return LastLookupFile;
+}
+
+DiagnosticsEngine::DiagStateMap::File *
+DiagnosticsEngine::DiagStateMap::getFileUncached(SourceManager &SrcMgr,
+                                                 FileID ID) const {
   // Get or insert the File for this ID.
   auto Range = Files.equal_range(ID);
   if (Range.first != Range.second)

``````````

</details>


https://github.com/llvm/llvm-project/pull/228956