[perf experiment] Split the resolver tables into per-owner tables#138995
[perf experiment] Split the resolver tables into per-owner tables#138995oli-obk wants to merge 8 commits intorust-lang:mainfrom
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[perf experiment] Split the resolver tables into per-owner tables r? `@ghost` just doing some experiments to see if splitting `hir_crate` is feasible by checking if splitting the resolver's output into per-owner queries is feasible (rust-lang#95004) Basically necessary for rust-lang#138705 as that can't be landed perf-wise while the `hir_crate` query is still a thing
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (792af13): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 1.3%, secondary 0.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 1.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 777.548s -> 776.554s (-0.13%) |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[perf experiment] Split the resolver tables into per-owner tables r? `@ghost` just doing some experiments to see if splitting `hir_crate` is feasible by checking if splitting the resolver's output into per-owner queries is feasible (rust-lang#95004) Basically necessary for rust-lang#138705 as that can't be landed perf-wise while the `hir_crate` query is still a thing
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (66f172c): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 1.6%, secondary 2.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 2.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 778.99s -> 777.791s (-0.15%) |
|
ok... better, but not great yet either |
|
@bors try @rust-timer queue |
[perf experiment] Split the resolver tables into per-owner tables r? `@ghost` just doing some experiments to see if splitting `hir_crate` is feasible by checking if splitting the resolver's output into per-owner queries is feasible (rust-lang#95004) Basically necessary for rust-lang#138705 as that can't be landed perf-wise while the `hir_crate` query is still a thing
This comment has been minimized.
This comment has been minimized.
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[perf experiment] Split the resolver tables into per-owner tables
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (225d1a5): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.8%, secondary 4.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.0%, secondary 7.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 480.915s -> 482.851s (0.40%) |
0959015 to
3e8279c
Compare
This comment has been minimized.
This comment has been minimized.
3e8279c to
1406df6
Compare
…ut for modification This is needed for follow-up cleanups as diagnostics still need access to the `NodeId` -> `DefId` map, but only for owners
It was only used for accessing owners anyway, so we can just look at the new owner table directly
1406df6 to
face229
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[perf experiment] Split the resolver tables into per-owner tables
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (e30e7bf): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.5%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.1%, secondary 5.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 496.246s -> 482.921s (-2.69%) |
View all comments
r? @ghost
Basically necessary for #138705 as that can't be landed perf-wise (1% primary, 2% secondary regressions) while the
hir_cratequery is still a thing.My goal is to split the resolver tables into per-owner tables, so that all information that ast lowering needs from the resolver is separated by owners. This should allow us to fully split ast lowering to have one query invocation per owner that steal the individual resolver results for each owner.
Alternatively we keep the entire resolver state around until hir_crate_items has run and steal and drop it then.