-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Avoid exhausting stack space in dominator compression #94306
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 4d89292 with merge f8e8e2794f33e98d8dabbb448963a624e7f5c2ee... |
☀️ Try build successful - checks-actions |
Queued f8e8e2794f33e98d8dabbb448963a624e7f5c2ee with parent 532d3cd, future comparison URL. |
Finished benchmarking commit (f8e8e2794f33e98d8dabbb448963a624e7f5c2ee): comparison url. Summary: This benchmark run did not return any relevant results. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking 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 led to changes in compiler perf. @bors rollup=never |
@bors rollup=maybe |
@bors r+ |
📌 Commit 4d89292 has been approved by |
Avoid exhausting stack space in dominator compression Doesn't add a test case -- I ended up running into this while playing with the generated example from rust-lang#43578, which we could do with a run-make test (to avoid checking a large code snippet into tree), but I suspect we don't want to wait for it to compile (locally it takes ~14s -- not terrible, but doesn't seem worth it to me). In practice stack space exhaustion is difficult to test for, too, since if we set the bound too low a different call structure above us (e.g., a nearer ensure_sufficient_stack call) would let the test pass even with the old impl, most likely. Locally it seems like this manages to perform approximately equivalently to the recursion, but will run perf to confirm.
…askrgr Rollup of 5 pull requests Successful merges: - rust-lang#93400 (Do not suggest using a const parameter when there are bounds on an unused type parameter) - rust-lang#93982 (Provide extra note if synthetic type args are specified) - rust-lang#94087 (Remove unused `unsound_ignore_borrow_on_drop`) - rust-lang#94235 (chalk: Fix wrong debrujin index in opaque type handling.) - rust-lang#94306 (Avoid exhausting stack space in dominator compression) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Doesn't add a test case -- I ended up running into this while playing with the generated example from #43578, which we could do with a run-make test (to avoid checking a large code snippet into tree), but I suspect we don't want to wait for it to compile (locally it takes ~14s -- not terrible, but doesn't seem worth it to me). In practice stack space exhaustion is difficult to test for, too, since if we set the bound too low a different call structure above us (e.g., a nearer ensure_sufficient_stack call) would let the test pass even with the old impl, most likely.
Locally it seems like this manages to perform approximately equivalently to the recursion, but will run perf to confirm.