-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Revert #107376 to fix potential bincode
breakage and rustc-perf
benchmark.
#109183
Conversation
I also think that this is probably the safest approach as well. |
Anyways, if you'd like to unblock perf sooner than later, r=me. |
That wasn't the primary motivation -- We're likely going to miss the nightly cutoff, unfortunately. We could add the MCVE as a test here, but if we plan to run crater anyways, it can be added when we revert this revert. Thanks @compiler-errors. r? @compiler-errors @bors r=compiler-errors |
bincode
breakage and rustc-perf
benchmark.
Revert rust-lang#107376 to fix potential `bincode` breakage and `rustc-perf` benchmark. rust-lang#107376 caused `rustc-perf`'s `webrender` benchmark to break, by regressing on the `bincode-1.3.3` crate. ~~This PR is a draft revert in case we can't land a fix soon enough, and we'd like to land the revert instead~~ (Though I myself think it'd be safer to do the revert, and run crater when relanding rust-lang#107376.) cc `@aliemjay`
@bors p=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the revert and the MCVE!
I haven't thought enough about how this blocks perf. I should've prioritized the issue more.
☀️ Test successful - checks-actions |
To be clear @aliemjay, perf.rlo wasn't blocked per se, the collector ignores benchmarks that fail but still measures the others. We just wouldn't see regressions or improvements to that specific crate. It also means that a real world crate was broken by a PR, in a way that CI didn't catch (but crater usually would) and that can be more problematic than the lack of benchmarking data for a short period of time. |
Finished benchmarking commit (18e305d): comparison URL. Overall result: ❌ regressions - 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)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. |
Even if no triage needs to be done per-se: this is a revert, and the |
#107376 caused
rustc-perf
'swebrender
benchmark to break, by regressing on thebincode-1.3.3
crate.This PR is a draft revert in case we can't land a fix soon enough, and we'd like to land the revert instead(Though I myself think it'd be safer to do the revert, and run crater when relanding #107376.)
cc @aliemjay