-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Vec::reserve(n) followed by n calls to push should only check capacity once #105156
Comments
Just ran into this too. Here's the generalization: pub fn insert(v: &mut Vec<usize>, n: usize) {
v.reserve(n);
for i in 0..n {
v.push(i);
}
}
pub fn create(n: usize) -> Vec<usize> {
let mut v = Vec::with_capacity(n);
for i in 0..n {
v.push(i);
}
v
} |
Fixed with this: pub fn insert(v: &mut Vec<usize>, n: usize) {
v.reserve(n);
for i in 0..n {
if v.len() >= v.capacity() {
unsafe { std::hint::unreachable_unchecked() }
}
v.push(i);
}
}
pub fn create(n: usize) -> Vec<usize> {
let mut v = Vec::with_capacity(n);
for i in 0..n {
if v.len() >= v.capacity() {
unsafe { std::hint::unreachable_unchecked() }
}
v.push(i);
}
v
} So should be handled by #82801 |
[Experiment] Eliminate possible `Vec::push` branches Related to rust-lang#105156. Requesting a perf run. ```rust pub fn push(v: &mut Vec<u8>) { let _ = v.reserve(4); v.push(1); v.push(2); v.push(3); v.push(4); } ``` AFAICT, the codegen backend should infer the infallibility of these `push`s but unfortunately with LLVM 18 we still have unnecessary `reserve_for_push` branches. For the sake of curiosity, `assert_unchecked` was included in `push` to see any potential impact of such change. Take a look at the generated assembly at https://godbolt.org/z/b5jjPhsf8. AFAICT (again), the assumption of more available capacity for each `push` is not valid for all situations.
An example that eliminates branches: https://godbolt.org/z/TTEv73o44 Change For whatever reason, the trick is in the use of |
I'd like to leave some more observations I've noticed while playing around with this: https://gist.github.com/cynecx/24e3a4ae8756fdf9257a6cbf50005e08 tl;dr: A single |
I tried this code in compiler explorer (https://rust.godbolt.org/z/W35vT4osM):
I expected the generated assembly to only check the capacity once, and reallocate if needed, and then write the value. Instead, for each
push
there is another check of remaining capacity and potential call toreserve_for_push
.Adding explicit
assume
calls leads to much shorter code with only a single call todo_reserve_and_handle
.This seems similar to #82801 but that issue seems specific to the initial vector capacity and code for that case looks ok now.
Meta
rustc --version --verbose
:The text was updated successfully, but these errors were encountered: