Skip to content

Conversation

@tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Nov 2, 2025

This is almost Arc::<str>::from(Box::<str>::from(owned_string)) that could avoid an extra copy.

Signed-off-by: tison <wander4096@gmail.com>
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 2, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 2, 2025

Currently it calls ptr::copy_nonoverlapping internally so there is still a copy. But it's semantically move and at least avoid iteratively calling ptr::write.

@hanna-kruppe
Copy link
Contributor

This implementation seems worse for copying: String::as_boxed_slice has to reallocate, i.e., copy, if there’s excess capacity, and Box<str> -> Arc<str> also has to reallocate to add refcounts. I don’t know what the current implementation compiles down to, but one copy/reallocation (for the refcounts and the alignment they imply) seems unavoidable.

@tisonkun tisonkun closed this Nov 3, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 3, 2025
@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 3, 2025

This implementation seems worse for copying: String::as_boxed_slice has to reallocate, i.e., copy, if there’s excess capacity, and Box<str> -> Arc<str> also has to reallocate to add refcounts. I don’t know what the current implementation compiles down to, but one copy/reallocation (for the refcounts and the alignment they imply) seems unavoidable.

Thanks for your analyze. I get the copy internal wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants