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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits