-
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 Vec/Rc storage reuse opt #104571
Revert Vec/Rc storage reuse opt #104571
Conversation
r? @m-ou-se (rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This doesn't seem to be a revert of #104205, any reason you didn't use git revert? Are there pieces that should be kept? |
The changes I kept were moving the |
Ok, looks good. Bumping prio since this should fix sporadic CI failures. @bors r+ p=1 |
☀️ Test successful - checks-actions |
Finished benchmarking commit (62c627c): 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.
|
This removes a recently-merged optimization that caused CI failures because it doesn't work reliably on all platforms. @rustbot label: +perf-regression-triaged |
Revert Vec/Rc storage reuse opt Remove the optimization for using storage added by rust-lang#104205. The perf wins were pretty small, and it relies on non-guarenteed behaviour. On platforms that don't implement shrinking in place, the performance will be significantly worse. While it could be gated to platforms that do this (such as GNU), I don't think it's worth the overhead of maintaining it for very small gains. (rust-lang#104565, rust-lang#104563) cc `@RalfJung` `@matthiaskrgr` Fixes rust-lang#104565 Fixes rust-lang#104563
Remove the optimization for using storage added by #104205.
The perf wins were pretty small, and it relies on non-guarenteed behaviour. On platforms that don't implement shrinking in place, the performance will be significantly worse.
While it could be gated to platforms that do this (such as GNU), I don't think it's worth the overhead of maintaining it for very small gains. (#104565, #104563)
cc @RalfJung @matthiaskrgr
Fixes #104565
Fixes #104563