-
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
Avoid a cast in ptr::slice_from_raw_parts(_mut)
#124795
Conversation
Casting to `*const ()` or `*mut ()` just bloats the MIR, so let's not. If ACP#362 goes through we can keep calling `ptr::from_raw_parts(_mut)` in these also without the cast, but that hasn't had any libs-api attention yet, so I'm not waiting on it.
Looks fine! Let's see what the performance improvements are... |
This comment has been minimized.
This comment has been minimized.
…ts, r=<try> Avoid a cast in `ptr::slice_from_raw_parts(_mut)` Casting to `*const ()` or `*mut ()` is no longer needed after rust-lang#123840 so let's make the MIR smaller (and more inline-able, as seen in the tests). If [ACP#362](rust-lang/libs-team#362) goes through we can keep calling `ptr::from_raw_parts(_mut)` in these also without the cast, but that hasn't had any libs-api attention yet, so I'm not waiting on it.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (8466125): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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 may lead to changes in compiler perf. @bors rollup=never 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 sizeResultsThis 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.
Bootstrap: 675.387s -> 674.274s (-0.16%) |
Lovely! |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ec1b698): comparison URL. Overall result: ❌✅ regressions and improvements - 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 sizeResultsThis 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.
Bootstrap: 675.214s -> 675.971s (0.11%) |
Nice binary size reductions here. |
Good to see that the 17.0% average cycle regression from the first perf run was fake. |
The wins far outweigh the single regression. @rustbot label: +perf-regression-triaged |
…=WaffleLapkin [ACP 362] genericize `ptr::from_raw_parts` This implements rust-lang/libs-team#362 As such, it can partially undo rust-lang#124795 , letting `slice_from_raw_parts` just call `from_raw_parts` again without re-introducing the unnecessary cast to MIR. By doing this it also removes a spurious cast from `str::from_raw_parts`. And I think it does a good job of showing the value of the ACP, since the only thing that needed new turbofishing because of this is inside `ptr::null(_mut)`, but only because `ptr::without_provenance(_mut)` doesn't support pointers to extern types, which it absolutely could (without even changing the implementation).
…=WaffleLapkin [ACP 362] genericize `ptr::from_raw_parts` This implements rust-lang/libs-team#362 As such, it can partially undo rust-lang#124795 , letting `slice_from_raw_parts` just call `from_raw_parts` again without re-introducing the unnecessary cast to MIR. By doing this it also removes a spurious cast from `str::from_raw_parts`. And I think it does a good job of showing the value of the ACP, since the only thing that needed new turbofishing because of this is inside `ptr::null(_mut)`, but only because `ptr::without_provenance(_mut)` doesn't support pointers to extern types, which it absolutely could (without even changing the implementation).
Rollup merge of rust-lang#125701 - scottmcm:generic-from-raw-parts, r=WaffleLapkin [ACP 362] genericize `ptr::from_raw_parts` This implements rust-lang/libs-team#362 As such, it can partially undo rust-lang#124795 , letting `slice_from_raw_parts` just call `from_raw_parts` again without re-introducing the unnecessary cast to MIR. By doing this it also removes a spurious cast from `str::from_raw_parts`. And I think it does a good job of showing the value of the ACP, since the only thing that needed new turbofishing because of this is inside `ptr::null(_mut)`, but only because `ptr::without_provenance(_mut)` doesn't support pointers to extern types, which it absolutely could (without even changing the implementation).
…pkin [ACP 362] genericize `ptr::from_raw_parts` This implements rust-lang/libs-team#362 As such, it can partially undo rust-lang/rust#124795 , letting `slice_from_raw_parts` just call `from_raw_parts` again without re-introducing the unnecessary cast to MIR. By doing this it also removes a spurious cast from `str::from_raw_parts`. And I think it does a good job of showing the value of the ACP, since the only thing that needed new turbofishing because of this is inside `ptr::null(_mut)`, but only because `ptr::without_provenance(_mut)` doesn't support pointers to extern types, which it absolutely could (without even changing the implementation).
Casting to
*const ()
or*mut ()
is no longer needed after #123840 so let's make the MIR smaller (and more inline-able, as seen in the tests).If ACP#362 goes through we can keep calling
ptr::from_raw_parts(_mut)
in these also without the cast, but that hasn't had any libs-api attention yet, so I'm not waiting on it.