Uh oh!
There was an error while loading. Please reload this page.
perf: avoid unnecessary large allocations - #22558
Conversation
cf4dd80 to
9f77401CompareThere was a problem hiding this comment.
@ariel-miculas
Thanks for working on this.
I do not see any blocking issues, just one small cleanup suggestion.
| } | ||
| } | ||
| #[cfg(test)] |
There was a problem hiding this comment.
Small cleanup idea: since this helper is only used by one test assertion, I think we can avoid adding the test-only values_capacity API.
The child tests module should be able to read the private field directly, so this could be simplified to:
let capacity_before = gv.values.capacity();// ...assert_eq!(gv.values.capacity(), capacity_before, ...);That would let us remove the #[cfg(test)] impl without changing behavior.
There was a problem hiding this comment.
thanks, applied your suggestion
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
## Which issue does this PR close? Related a bit to apache#22526 ~Needs rebasing once apache#22416 is merged~ ## Rationale for this change split_off does this: > Returns a newly allocated vector containing the elements in the range [at, len). After the call, the original vector will be left containing the elements [0, at) with its previous capacity unchanged. which is bad when taking a small slice from a large Vec, for two reasons: * it will allocate memory for the remaining elements, which are a lot more than n * it will return a Vec with a very large capacity compared to its length split_vec_min_alloc still has some issues: apache#22548 but it uses drain + collect when n is small, which is better because it only allocates for the initial n elements and doesn't inflate the capacity ## What changes are included in this PR? ## Are these changes tested? Yes ## Are there any user-facing changes? No (cherry picked from commit c1f0d54)
Which issue does this PR close?
Related a bit to #22526
Needs rebasing once #22416 is mergedRationale for this change
split_off does this:
which is bad when taking a small slice from a large Vec, for two reasons:
split_vec_min_alloc still has some issues: #22548 but it uses drain + collect when n is small, which is better because it only allocates for the initial n elements and doesn't inflate the capacity
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?
No