mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-09-24 20:50:13 +01:00
Jon Ross-Perkins
0683742f19
Right now we construct `tree_and_subtrees_getters` a couple different ways, it's just not obvious because one's abstracted in `check`. But also, when formatting IR, we'll repeatedly do the `IncludeInDumps` string check, which felt odd to me since it only needs to be calculated once per IR. This also shifts `CheckIRId` selection a little earlier, and in doing so makes `CheckParseTrees` accept a sparse `units` argument. I actually think this is a positive: it makes `CheckIRId` a little more stable across possible command lines, when file loading fails (which is the only time that a file will have a `CompilationUnit` but not a `Check::Unit`). Trying to build on the shared issue between these, I'm adding a `MultiUnitCache` to store the calculated arrays. For the subtree getters, this is very minor and avoids at most one incremental array construction (moving logic out of `CompileSubcommand::Run` might be the bigger benefit). For `include_in_dumps`, when dumping SemIR, this is changing a calculation run once per entity (in each IR) to be calculated once per IR (globally), i.e. O(M*N) -> O(N). Note this seems to be marginal for performance of file_test: - Before: Stats over 10 runs: max = 5.3s, min = 4.7s, avg = 4.9s, dev = 0.2s - After: Stats over 10 runs: max = 4.9s, min = 4.7s, avg = 4.8s, dev = 0.1s I was mainly thinking about this in the context of dumping SemIR ranges. There, the impact may actually decrease because a range won't do any cross-IR printing. But, I'm expecting to add another layer for whether we're printing IR for a file, and that made the `should_format_entity` callback stick out for me.