[PATCH] D106876: Remove non-affecting module maps from PCM files.

Wed Oct 19 15:14:35 PDT 2022

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