[clang] 408efff - [Clang][SourceManager] optimize getFileIDLocal()

Nick Desaulniers via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 25 09:59:58 PDT 2020


Author: Nick Desaulniers
Date: 2020-06-25T09:59:41-07:00
New Revision: 408efffbe4a52bae05f1677a47eb3ccfd5cdc1d3

URL: https://github.com/llvm/llvm-project/commit/408efffbe4a52bae05f1677a47eb3ccfd5cdc1d3
DIFF: https://github.com/llvm/llvm-project/commit/408efffbe4a52bae05f1677a47eb3ccfd5cdc1d3.diff

LOG: [Clang][SourceManager] optimize getFileIDLocal()

Summary:
A recent Linux kernel commit exposed a performance cliff in Clang. Calls
to SourceManager::getFileIDLocal() when there's a cache miss against
LastFileIDLookup can be relatively expensive, as getFileIDLocal() tries
a few linear probes, then falls back to binary search.  The use of
SourceManager::isOffsetInFileID() is also relatively expensive (both
isOffsetInFileID and getFileIDLocal dominated a trace of the performance
cliff case).

As a FIXME notes (and as @kadircet helpfully noted in review of D80681),
there's a few optimizations we can do here since we've already
identified that an offset is local (as opposed to "loaded").

This patch was forked off of D80681, which additionally did this and
modified some caching behavior, as we expect this change to be less
controversial.

In terms of optimizations, we've already determined that the SLocOffset
parameter to SourceManager::getFileIDLocal() is local in the caller
SourceManager::getFileIDSlow(). Also, there's an early continue in the
binary search loop in getFileIDLocal() that are duplicated in
isOffsetInFileID() as pointed out by @kadircet.

Take advantage of these to optimize the binary search patch, and remove
the FIXME.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: cfe-commits, kadircet, srhines

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82497

Added: 
    

Modified: 
    clang/lib/Basic/SourceManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 6e4c8e8052bd..63518bf8b79a 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -898,9 +898,8 @@ FileID SourceManager::getFileIDLocal(unsigned SLocOffset) const {
     }
 
     // If the middle index contains the value, succeed and return.
-    // FIXME: This could be made faster by using a function that's aware of
-    // being in the local area.
-    if (isOffsetInFileID(FileID::get(MiddleIndex), SLocOffset)) {
+    if (MiddleIndex + 1 == LocalSLocEntryTable.size() ||
+        SLocOffset < getLocalSLocEntry(MiddleIndex + 1).getOffset()) {
       FileID Res = FileID::get(MiddleIndex);
 
       // If this isn't a macro expansion, remember it.  We have good locality


        


More information about the cfe-commits mailing list