Skip to content
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

cstring: avoid excessive growth just to 0-terminate #35871

Merged
merged 1 commit into from
Aug 22, 2016

Conversation

bluss
Copy link
Member

@bluss bluss commented Aug 21, 2016

Based on following what happens in CString::new("string literal"):

  1. Using Into<Vec<u8>>, a Vec is allocated with capacity exactly equal
    to the string's input length.
  2. By v.push(0), the Vec is grown to twice capacity, since it was full.
  3. By v.into_boxed_slice(), the Vec capacity is shrunk to fit the length again.

If we use .reserve_exact(1) just before the push, then we avoid the
capacity doubling that we're going to have to shrink anyway.

Growing by just 1 byte means that the step (2) is less likely to have to
move the memory to a larger allocation chunk, and that the step (3) does
not have to reallocate.

Addresses part of #35838

Based on following what happens in CString::new("string literal"):

1. Using `Into<Vec<u8>>`, a Vec is allocated with capacity exactly equal
   to the string's input length.
2. By `v.push(0)`, the Vec is grown to twice capacity, since it was full.
3. By `v.into_boxed_slice()`, the Vec capacity is shrunk to fit the length again.

If we use `.reserve_exact(1)` just before the push, then we avoid the
capacity doubling that we're going to have to shrink anyway.

Growing by just 1 byte means that the step (2) is less likely to have to
move the memory to a larger allocation chunk, and that the step (3) does
not have to reallocate.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@retep998
Copy link
Member

The Into<Vec<u8>> part is a shame though, as it forces our hand in still having to potentially reallocate for that null byte, although since now it is just 1 more byte, it is much more likely to be able to reallocate inplace, so it might not be terrible in practice.

@bluss
Copy link
Member Author

bluss commented Aug 21, 2016

That can be addressed by specialization (I imagine, IntoVecWithOneExtraCapacity), but that feels much more messy, not sure it's worth the complexity

@alexcrichton
Copy link
Member

@bors: r+

Thanks @bluss! Seems like a good optimization to have regardless for now at leaset.

@bors
Copy link
Contributor

bors commented Aug 21, 2016

📌 Commit 876c02c has been approved by alexcrichton

eddyb added a commit to eddyb/rust that referenced this pull request Aug 22, 2016
cstring: avoid excessive growth just to 0-terminate

Based on following what happens in CString::new("string literal"):

1. Using `Into<Vec<u8>>`, a Vec is allocated with capacity exactly equal
   to the string's input length.
2. By `v.push(0)`, the Vec is grown to twice capacity, since it was full.
3. By `v.into_boxed_slice()`, the Vec capacity is shrunk to fit the length again.

If we use `.reserve_exact(1)` just before the push, then we avoid the
capacity doubling that we're going to have to shrink anyway.

Growing by just 1 byte means that the step (2) is less likely to have to
move the memory to a larger allocation chunk, and that the step (3) does
not have to reallocate.

Addresses part of rust-lang#35838
@bors
Copy link
Contributor

bors commented Aug 22, 2016

⌛ Testing commit 876c02c with merge 3c5a0fa...

bors added a commit that referenced this pull request Aug 22, 2016
cstring: avoid excessive growth just to 0-terminate

Based on following what happens in CString::new("string literal"):

1. Using `Into<Vec<u8>>`, a Vec is allocated with capacity exactly equal
   to the string's input length.
2. By `v.push(0)`, the Vec is grown to twice capacity, since it was full.
3. By `v.into_boxed_slice()`, the Vec capacity is shrunk to fit the length again.

If we use `.reserve_exact(1)` just before the push, then we avoid the
capacity doubling that we're going to have to shrink anyway.

Growing by just 1 byte means that the step (2) is less likely to have to
move the memory to a larger allocation chunk, and that the step (3) does
not have to reallocate.

Addresses part of #35838
@bors bors merged commit 876c02c into rust-lang:master Aug 22, 2016
@bluss bluss deleted the cstring-new branch August 22, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants