-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cache type sizes in type-size limit visitor #127288
Conversation
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
cache type sizes in type-size limit visitor This is rust-lang#125507 (comment) as lcnr can't open the PR now. Locally it reduces the `itertools` regression by quite a bit, "only +50%" compared to nightly. ```console Benchmark 1: cargo +stage1 build --release Time (mean ± σ): 2.721 s ± 0.009 s [User: 2.446 s, System: 0.325 s] Range (min … max): 2.710 s … 2.738 s 10 runs Benchmark 2: cargo +nightly build --release Time (mean ± σ): 1.784 s ± 0.005 s [User: 1.540 s, System: 0.279 s] Range (min … max): 1.778 s … 1.792 s 10 runs Summary cargo +nightly build --release ran 1.52 ± 0.01 times faster than cargo +stage1 build --release ``` On master, it's from 34s to the 2.7s above. r? compiler-errors just as a cc, as they said they might open the same PR later Opening as draft to do a perf run to see `deeply-nested-multi` (which should be fixed on the perf server now), and validate bootstrap times.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
The joys of @bors try |
cache type sizes in type-size limit visitor This is rust-lang#125507 (comment) as lcnr can't open the PR now. Locally it reduces the `itertools` regression by quite a bit, "only +50%" compared to nightly. ```console Benchmark 1: cargo +stage1 build --release Time (mean ± σ): 2.721 s ± 0.009 s [User: 2.446 s, System: 0.325 s] Range (min … max): 2.710 s … 2.738 s 10 runs Benchmark 2: cargo +nightly build --release Time (mean ± σ): 1.784 s ± 0.005 s [User: 1.540 s, System: 0.279 s] Range (min … max): 1.778 s … 1.792 s 10 runs Summary cargo +nightly build --release ran 1.52 ± 0.01 times faster than cargo +stage1 build --release ``` On master, it's from 34s to the 2.7s above. r? compiler-errors just as a cc, as they said they might open the same PR later Opening as draft to do a perf run to see `deeply-nested-multi` fixed on the perf server (the type length has been bumped), and validate bootstrap times.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (83217ae): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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 a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary 2.0%, secondary 2.6%)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 (primary 1.8%, secondary -99.0%)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: 742.585s -> 701.096s (-5.59%) |
This should be ready to review, as at least a first step to reduce the problem.
@rustbot ready |
comparison url using the start of #125507's perf run and the end is this commit. Looks like we fixed everything unless I just can't read? |
@bors r+ rollup=never |
☀️ Test successful - checks-actions |
Finished benchmarking commit (cc8da78): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary -2.0%, secondary 1.0%)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 -98.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: 721.419s -> 698.879s (-3.12%) |
This is basically #125507 (comment) as lcnr can't open the PR now.
Locally it reduces the
itertools
regression by quite a bit, to "only +50%" compared to nightly (that includes overhead from the local lack of artifact post-processing, and is just a data point to compare to the 10-20x timings without the cache).On master, it's from 34s to the 2.7s above.
r? compiler-errors