-
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
Don't use eval_always for miri queries used from codegen. #65927
Conversation
@bors rollup=never p=10 (ideally we can land it in time for the next nightly) |
@bors r+ |
📌 Commit beb06ae has been approved by |
…lwoerister Don't use eval_always for miri queries used from codegen. This should fix the [massive incremental perf regression](https://perf.rust-lang.org/compare.html?start=95f437b3cfb2fec966d7eaf69d7c2e36f9c274d1&end=9285d401a6070094747465962bc49969b93e14c5&stat=instructions:u) introduced in #65664. It seems that `eval_always` was mistakenly(?) added to `const_field` and then it ended up on `const_caller_location` (which is used much more often than `const_field` is). r? @michaelwoerister cc @oli-obk @nnethercote
☀️ Test successful - checks-azure |
@rust-timer build eb5ef81 (I'm trying to prioritize the build just before this PR, to get the perf diff data sooner) |
Queued eb5ef81 with parent 2dd4e73, future comparison URL. |
Comparisons I'm interested in:
|
If you look at just the
So the |
Wait, check? From #65664? Are there a lot of We went the way we did was EDIT: opened #65973 for that |
2-4% is non-trivial, so if there's a way to recover from it that would be great. |
This comment has been minimized.
This comment has been minimized.
Interestingly enough, |
…ochenkov caller_location: point to macro invocation sites, like file!/line!, and use in core::panic!. The main change here is to `core::panic!`, trying to fix this remaining regression: rust-lang#65927 (comment) However, in order for `caller_location` to be usable from macros the same way `file!()`/`line!()` are, it needs to have the same behavior (of extracting the macro invocation site `Span` and using that). Arguably we would've had to do this at some point anyway, if we want to use `#[track_caller]` to replace the `file!()`/`line!()` uses from macros, but I'm not sure the RFC mentions this at all. r? @petrochenkov cc @anp @nnethercote
This should fix the massive incremental perf regression introduced in #65664.
It seems that
eval_always
was mistakenly(?) added toconst_field
and then it ended up onconst_caller_location
(which is used much more often thanconst_field
is).r? @michaelwoerister cc @oli-obk @nnethercote