-
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
Rollup of 3 pull requests #118982
Rollup of 3 pull requests #118982
Conversation
Add some reasons to the panics, and use more exhaustive matches.
Annotate some bugs Gives a semi-helpful message to some `bug!()`/`unreachable!()`/`panic!()`. This also works around some other bugs/panics/etc that weren't needed, and also makes some of them into `span_bug!`s so they also have a useful span. Note to reviewer: best to disable whitespace when comparing for some cases where indentation changed. cc rust-lang#118955
…rors coverage: Use `Waker::noop` in async tests Inspired by rust-lang#118948. --- `@rustbot` label +A-code-coverage
…m-abi, r=davidtwco Annotate panic! reasons during enum layout Add some reasons to the panics, and use more exhaustive matches. Also see: rust-lang#118955
@bors r+ rollup=never p=5 |
☀️ Test successful - checks-actions |
📌 Perf builds for each rolled up PR:
previous master: 4d1bd0db7f In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
Finished benchmarking commit (e6707df): comparison URL. Overall result: ❌ regressions - 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)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.
CyclesResultsThis 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: 674.139s -> 673.805s (-0.05%) |
@rust-timer build 50c2fd8 |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (50c2fd8): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDInstruction 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.
CyclesResultsThis 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: 674.139s -> 672.369s (-0.26%) |
cc @compiler-errors It seems your annotation PR in #118962 has caused a nontrivial perf regression. On the other hand, the only "real world" bench affected is "Hello, world!", and all the other ones are stress benches, so perhaps this is more artificial than it seems. Either way, I suspect you'd have a better sense for evaluating this. |
Huh, I don't expect to have done anything in that PR resulting in worse performance... |
I think that this is just a continuation of the recent noise spikes in some benchmarks. CC @lqd who can best recognize these situations. |
...Surely there is some sort of statistical method to isolate "noise" like this? |
I also believe it's noise. My theory is that the noise threshold gets lower due to a period of lesser noise, and when the usual noise comes back this is seen as a regression. It's what I imagine the graph shows, but we probably would be able to tell better if we saved a history of the threshold, and visualized it as a shaded range on the above graph. edit: the changes here went back to their steady-state in the PR that was merged immediately afterwards. Marking as triaged due to noise. @rustbot label: +perf-regression-triaged |
Successful merges:
Waker::noop
in async tests #118969 (coverage: UseWaker::noop
in async tests)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup