Add some methods and optimizations in buffer vec#22470
Add some methods and optimizations in buffer vec#22470alice-i-cecile merged 1 commit intobevyengine:mainfrom
Conversation
Add `reserve_internal` to `BufferVec` Add `capacity` `set_label` `get_label` to `UninitBufferVec` Use `Vec::reserve` to reduce some allocation
| /// Preallocates space for `count` elements in the internal CPU-side buffer. | ||
| /// | ||
| /// Unlike [`Self::reserve`], this doesn't have any effect on the GPU buffer. | ||
| pub fn reserve_internal(&mut self, count: usize) { |
There was a problem hiding this comment.
Any reason why you don't call this inside .reserve()? It seems to me like it would make sense to do that?
There was a problem hiding this comment.
RawBufferVec::reserve_internal has use cases where the creation/writing of GPU buffers is deferred.
| let element_size = u64::from(T::min_size()) as usize; | ||
| let offset = self.data.len(); | ||
|
|
||
| // `extend` does not optimize for reallocation. Related `trusted_len` feature is unstable. |
There was a problem hiding this comment.
I don't understand the part about trusted_len
There was a problem hiding this comment.
Vec.extend reserves space internally if iterator impls TrustedLen trait (RepeatN does), though I haven't tested this.
There was a problem hiding this comment.
Did not know that, it's like a stronger size_hint iiuc.
|
|
||
| // `extend` does not optimize for reallocation. Related `trusted_len` feature is unstable. | ||
| self.data.reserve(self.data.len() + element_size); | ||
| // TODO: Consider using unsafe code to push uninitialized, to prevent |
There was a problem hiding this comment.
My suggestion (after #22361) would be to reword the the following TODO comment:
// We can't optimize and push uninitialized data here because write_into() does not initialize any T's inner paddings
# Objective In #22470 (comment) I thought `Vec::extend(repeat_n( ... ))` doesn't have `TrustedLen` optimization but I was wrong! Rust std is allowed to use unstable features internally. See https://godbolt.org/z/x4sGj9on9 the generated MIR uses `Vec::extend_trusted`. ## Solution Remove the unnecessary `reserve` ## Testing None
# Objective In bevyengine#22470 (comment) I thought `Vec::extend(repeat_n( ... ))` doesn't have `TrustedLen` optimization but I was wrong! Rust std is allowed to use unstable features internally. See https://godbolt.org/z/x4sGj9on9 the generated MIR uses `Vec::extend_trusted`. ## Solution Remove the unnecessary `reserve` ## Testing None
Objective
UninitBufferVecis missing methods to get its capacity and set label. AndBufferVecneedsreserve_internalto reduce memory reallocation if element count is known.Also there are two places that we can use
Vec::reserveto preallocate memory.Solution
Add
reserve_internaltoBufferVecAdd
capacityset_labelget_labeltoUninitBufferVecUse
Vec::reserveto reduce some allocationTesting
CI