-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use a reduced recursion limit in the MIR inliner's cycle breaker #129714
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
e988100
to
950437a
Compare
@bors r+ rollup=never |
Ideally we'd have a UI test for this, but presumably it depends too much on the def path ordering hack? Or were you just too lazy to add a test, @saethlin? |
I'm too lazy, but also you're right that a UI test would be probabilistic, though we have at least one of those already. |
@saethlin: In the future please make sure to bundle observable changes to the compiler with a UI test if possible, even if it's probabilistic :^) |
I can probably add a ui test to this tomorrow |
@bors p=1 Candidate for backport to 1.81. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (784d444): comparison URL. Overall result: ✅ improvements - no action needed@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)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: missing data |
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
Something weird has happened here. Bootstrap was not benchmarked and only a subset of the normal benchmarks was executed. In the following merge, bootstrap was measured fine, but we also only got a subset of results, because it was compared with this PR. The next merged PR should show us whether it was a transient issue. |
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[beta] backports - Emit specific message for `time<0.3.35` inference failure rust-lang#129343 - Use a reduced recursion limit in the MIR inliner's cycle breaker rust-lang#129714 - rustdoc: do not run doctests with invalid langstrings rust-lang#128838 r? cuviper
[do not merge] test rust-lang#129714 revert on bootstrap times It seems we're currently experiencing some instability with the benchmarking results. rust-lang#129714 had weird results, somehow failing to measure bootstrap and other benchmarks. Since then bootstrap measurements [have also increased](https://perf.rust-lang.org/bootstrap.html). Let's see if a revert does anything to bootstrap times at the very least, to see if the PR is maybe involved. r? ghost
[do not merge] test rust-lang#129714 revert on bootstrap times It seems we're currently experiencing some instability with the benchmarking results. rust-lang#129714 had weird results, somehow failing to measure bootstrap and other benchmarks. Since then bootstrap measurements [have also increased](https://perf.rust-lang.org/bootstrap.html). Let's see if a revert does anything to bootstrap times at the very least, to see if the PR is maybe involved. r? ghost
Finished benchmarking commit (784d444): comparison URL. Overall result: ✅ improvements - no action needed@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.2%, secondary 0.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.
CyclesResults (secondary -1.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.
Binary sizeResults (secondary -0.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.
Bootstrap: 791.88s -> 787.871s (-0.51%) |
This probably papers over #128887, but primarily I'm opening this PR because multiple compiler people have thought about making this change which probably means it's a good idea.
r? compiler-errors