[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D20401#3833201 <https://reviews.llvm.org/D20401#3833201>, @hokein wrote:

>>> Meanwhile, I think besides evaluating the high level logic in in TokenLexer and how it might be improved, I think there's potentially an opportunity for a "AOS vs. SOA" speedup in SourceManager. SourceManager::LoadedSLocEntryTable is a llvm::SmallVector<SrcMgr::SLocEntry>. SourceManager::getFileIDLoaded only really cares about the SLocEntry's Offset. I suspect we could get major memory locality wins by packing those into a standalone vector so that we could search them faster.
>>
>> Ah, great point. SLocEntry is 24 bytes while Offset is only 4.
>
> And SLocEntry costs 4 bytes for padding only, which is bad :(
>
>> SLocEntry is an important public API, but Offset is ~only used in SourceManager, so that refactoring might be doable. I guess we can cheaply prototype by redundantly storing offset *both* in a separate array used for search and in the SLocEntry.

Do we need to store both the whole SLocEntry and a copy of the Offset, or can we just store the Offset (or perhaps individual arrays of the pieces of an SLocEntry)? Perhaps we can lazily materialize an SLocEntry only when needed, if ever?

> This is an interesting idea. I got a quick prototype of adding an in-parallel offset table in SourceManager:
>
> - clang::SourceManager::getFileIDLocal  2.45% -> 1.57% (reduce by 30%+)
> - SourceManager memory usage is increased by ~10%:  `SemaExpr.cpp` 12.6MB -> 14.3MB

How did you measure the memory usage of an individual class? (I think we should move this discussion to LLVM Discourse for more visibility of our discussion).

> The improvement of `getFileIDLocal` seems promising, but the memory increasement is a thing (10% is not small, maybe it is ok compared the actual AST size).

At this point, I'll pay it.  Unless it regresses peak RSS of the compiler, I don't care.

> An alternative is to restructure the SLocEntry and the underlying storage in SourceManager, it will give us both performance and memory improvement, but we need to make a significant change of the SourceManager.

At this point, I think it's worth it.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D20401/new/

https://reviews.llvm.org/D20401