[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith added a comment. In D89749#2356846 , @v.g.vassilev wrote: > Is the performance overhead for loaded module a % of the overall size of the > source files? `SourceManager::PrintStats` almost has the information you need. One of the lines is:

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

v.g.vassilev added a reviewer: rsmith. v.g.vassilev added a comment. Unfortunately, the patch does not apply against llvm9 (which is what we have as experimental setup) not to speak against llvm5 which is our production setup. Is the performance overhead for loaded module a % of the overall size

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith added a comment. In D89749#2354386 , @dexonsmith wrote: > That means it's not safe to store an address from `getSLocEntry` when there > will be another call. I can update `ASTImporter` to use a copy, but I think > this is too much of a gotcha.

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith added a comment. @teemperor, I don't get test failures: % (cd build && env LIT_FILTER=Modules ninja -k20 check-lldb) ld: warning: spew Testing Time: 10.54s Excluded : 2105 Unsupported: 17 Passed : 13 Could this be because I used `-DLLDB_USE_SYSTEM_DEBUGS

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith added a comment. In D89749#2353602 , @teemperor wrote: > The tests are just loading a Clang module and then trying to import some > Decls into another ASTContext, so I don't think the error here is specific to > LLDB. We just don't have any AS

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith planned changes to this revision. dexonsmith added a comment. (still need to investigate ASTImporter) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749 ___ cfe-commits mailing list cfe-commits

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith updated this revision to Diff 300740. dexonsmith added a comment. Fix name of `LoadedSLocEntryIndices` (I documented it with this name, but was using `SLocEntryIndices` previously). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749 Fil

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith updated this revision to Diff 300737. dexonsmith added a comment. Added a unit test for `LoadedSLocEntryIndex`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749 Files: clang/include/clang/Basic/SourceManager.h clang/lib/Basic/Sourc

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith planned changes to this revision. dexonsmith added a comment. In D89749#2354072 , @dexonsmith wrote: > I still need to investigate the LLDB test failures related to `ASTImporter`. (still to-do) CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith planned changes to this revision. dexonsmith added a comment. I still need to investigate the LLDB test failures related to `ASTImporter`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749 ___

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith updated this revision to Diff 300722. dexonsmith edited the summary of this revision. dexonsmith added a comment. Made the planned changes to SourceManager, wrapping the index in `LoadedSLocEntryIndex` which acts like an `Optional` (but still using 0 for a sentinel to allow zero-initi

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

teemperor added a comment. >> The tests are just loading a Clang module and then trying to import some >> Decls into another ASTContext, so I don't think the error here is specific >> to LLDB. We just don't have any ASTImporter tests that exercise modules. > > I'll add one. I'm actually not eve

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith planned changes to this revision. dexonsmith added a comment. In D89749#2353651 , @v.g.vassilev wrote: > Thanks for the patch!! This is a super hot place for us (mostly due to > boost). I will try it on our end and let you know! Great! In D8

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

v.g.vassilev added a comment. Thanks for the patch!! This is a super hot place for us (mostly due to boost). I will try it on our end and let you know! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

teemperor added a subscriber: v.g.vassilev. teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Maybe arc didn't correctly apply the patch, but this causes a few LLDB tests to fail on my machine: lldb-api :: commands/expressi

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith added a comment. This should be complementary to https://reviews.llvm.org/D89580, but I think it's independent. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89749/new/ https://reviews.llvm.org/D89749 ___ cfe-commits mailing list

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

dexonsmith created this revision. dexonsmith added reviewers: teemperor, Bigcheese. Herald added a subscriber: ributzka. dexonsmith requested review of this revision. Convert `SLocEntryLoaded` into an index vector into `LoadedSLocEntryTable`, where 0 indicates "not loaded", and change `LoadedSLocE