dexonsmith added a comment. In D106876#3869447 <https://reviews.llvm.org/D106876#3869447>, @jansvoboda11 wrote:
> I agree that avoiding serializing non-affecting input files is the better > approach. Your code is more or less what I had in mind, thanks for sketching > it out! > The number of ignored module maps is typically around 70 - 110 on macOS (when > you allow system module maps to be treated as non-affecting), and those are > at the start of the `SourceManager` block. I might implement this approach > and test out if there's a noticeable performance impact. Maybe we could use > something similar to the optimization in `SourceManager::getFileIDLoaded()`. Interesting. In that case it'd probably be profitable to do a pass after sorting to merge adjacent SourceLoc ranges. If there are many ignored system modules they may well be immediately adjacent and they'd resolve to a single range. > I remember having a separate `SourceManager` came up before, when > investigating other issues. Though I think you want to merge `SourceManager`s > earlier than on serialization. I think other data structures might hold a > `SourceLocation` pointing into the module-map-specific `SourceManager`. How > can you tell which `SourceManager` it came from? I'm curious whether this was a problem before, when there were two SourceManagers (before they were combined into one). It's possible that it's just obvious from context which `SourceManager` to use; that there isn't ambiguity in practice (because of where the SourceLocs are stored, or the source language, or something?). Note that fully splitting the source manager might be worth doing (eventually) as a follow up, even if there aren't performance problems with the remapping approach. (Unless this patch-and-the-bug-fixes will address all the performance problems? I don't think it totally solves the quadratic use of SourceLocation bit space by module maps, but maybe it does?) > This could be prevented by merging the module-map-specific `SourceManager` > into the main one when returning non-null result from `Module > *ModuleMap::lookupModule(StringRef Name)`, were that the only way to get a > module. Ah, yeah, on-demand, but sooner than I was thinking. SGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
