Alloc String::retain optimization#150067
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Alloc `String::retain` optimization
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (2621d01): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.1%, secondary 0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 68.5%, secondary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 479.038s -> 478.434s (-0.13%) |
|
r? joboet Probably makes sense to have the same reviewer for both. |
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bc23951 to
875c63d
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
66f0f66 to
2af59e0
Compare
|
Reminder, once the PR becomes ready for a review, use |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
|
⌛ Trying commit 590ba93 with merge 886f85e… To cancel the try build, run the command Workflow: https://github.com/rust-lang/rust/actions/runs/20782421026 |
Alloc `String::retain` optimization
| let string_ptr = ptr::NonNull::from(&mut *self); | ||
| let data_ptr = self.vec.as_mut_ptr(); | ||
| let mut chars = self.char_indices(); |
There was a problem hiding this comment.
This is unsound – data_ptr and string_ptr are both invalidated by the call to char_indices, as that creates a mutable reference to self and the str.
| // Critical section starts here, at least one character is going to be removed. | ||
| let mut g = PanicGuard { s: string_ptr, read, write }; | ||
| // Slower write-path | ||
| while let Some((read, ch)) = chars.next() { |
There was a problem hiding this comment.
You'll thus need to create the CharIndices iterator on every iteration.
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
Hi again!
This uses the exact same algorithm as my other PR: #149784
Technically it should improve performance for
String::retaintoo, But let's see what bors thinks.