Skip to content

[clang] Speedup getFileIDLocal with a separate offset table. #146604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −7
Open

[clang] Speedup getFileIDLocal with a separate offset table. #146604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/SourceManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,8 @@ class SourceManager : public RefCountedBase<SourceManager> {
/// Positive FileIDs are indexes into this table. Entry 0 indicates an invalid
/// expansion.
SmallVector<SrcMgr::SLocEntry, 0> LocalSLocEntryTable;
/// An in-parallel offset table, merely used for speeding up FileID lookup.
SmallVector<SourceLocation::UIntTy> LocalLocOffsetTable;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the size/cache sensitivity of this, might it be better to not use the SmallVector and either use a normal std::vector (no small-vector opt, see the table above does this with the , 0), or a very large small-vector-opt here? Do we have a good idea what size these usually do?


/// The table of SLocEntries that are loaded from other modules.
///
Expand Down
16 changes: 9 additions & 7 deletions clang/lib/Basic/SourceManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ SourceManager::~SourceManager() {
void SourceManager::clearIDTables() {
MainFileID = FileID();
LocalSLocEntryTable.clear();
LocalLocOffsetTable.clear();
LoadedSLocEntryTable.clear();
SLocEntryLoaded.clear();
SLocEntryOffsetLoaded.clear();
Expand Down Expand Up @@ -628,9 +629,11 @@ FileID SourceManager::createFileIDImpl(ContentCache &File, StringRef Filename,
noteSLocAddressSpaceUsage(Diag);
return FileID();
}
assert(LocalSLocEntryTable.size() == LocalLocOffsetTable.size());
LocalSLocEntryTable.push_back(
SLocEntry::get(NextLocalOffset,
FileInfo::get(IncludePos, File, FileCharacter, Filename)));
LocalLocOffsetTable.push_back(NextLocalOffset);
LastLookupStartOffset = NextLocalOffset;
// We do a +1 here because we want a SourceLocation that means "the end of the
// file", e.g. for the "no newline at the end of the file" diagnostic.
Expand Down Expand Up @@ -684,7 +687,9 @@ SourceManager::createExpansionLocImpl(const ExpansionInfo &Info,
SLocEntryLoaded[Index] = SLocEntryOffsetLoaded[Index] = true;
return SourceLocation::getMacroLoc(LoadedOffset);
}
assert(LocalSLocEntryTable.size() == LocalLocOffsetTable.size());
LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
LocalLocOffsetTable.push_back(NextLocalOffset);
if (NextLocalOffset + Length + 1 <= NextLocalOffset ||
NextLocalOffset + Length + 1 > CurrentLoadedOffset) {
Diag.Report(diag::err_sloc_space_too_large);
Expand Down Expand Up @@ -807,6 +812,7 @@ FileID SourceManager::getFileIDLocal(SourceLocation::UIntTy SLocOffset) const {
assert(SLocOffset < NextLocalOffset && "Bad function choice");
assert(SLocOffset >= LocalSLocEntryTable[0].getOffset() && SLocOffset > 0 &&
"Invalid SLocOffset");
assert(LocalSLocEntryTable.size() == LocalLocOffsetTable.size());
assert(LastFileIDLookup.ID >= 0 && "Only cache local file sloc entry");

// After the first and second level caches, I see two common sorts of
Expand Down Expand Up @@ -837,8 +843,8 @@ FileID SourceManager::getFileIDLocal(SourceLocation::UIntTy SLocOffset) const {
unsigned NumProbes = 0;
while (true) {
--GreaterIndex;
assert(GreaterIndex < LocalSLocEntryTable.size());
if (LocalSLocEntryTable[GreaterIndex].getOffset() <= SLocOffset) {
assert(GreaterIndex < LocalLocOffsetTable.size());
if (LocalLocOffsetTable[GreaterIndex] <= SLocOffset) {
FileID Res = FileID::get(int(GreaterIndex));
// Remember it. We have good locality across FileID lookups.
LastFileIDLookup = Res;
Expand All @@ -858,11 +864,7 @@ FileID SourceManager::getFileIDLocal(SourceLocation::UIntTy SLocOffset) const {
++NumBinaryProbes;

unsigned MiddleIndex = LessIndex + (GreaterIndex - LessIndex) / 2;

SourceLocation::UIntTy MidOffset =
LocalSLocEntryTable[MiddleIndex].getOffset();

if (MidOffset <= SLocOffset)
if (LocalLocOffsetTable[MiddleIndex] <= SLocOffset)
LessIndex = MiddleIndex + 1;
else
GreaterIndex = MiddleIndex;
Expand Down
Loading