Skip to content

Commit 4eaef66

Browse files
Dunqingoverlookmotel
authored andcommitted
perf(allocator/vec2): align min amortized cap size with std (oxc-project#9857)
Align with https://doc.rust-lang.org/src/alloc/raw_vec.rs.html#653-656, but unfortunately, we got a performance regression from this optimization, which was caused by `let cap = cmp::max(Self::MIN_NON_ZERO_CAP, cap);`. So I commented it out and added comments to describe why. Although the performance has not changed, keep the implementation the same as the standard library is also nice to have.
1 parent 3cd3d23 commit 4eaef66

File tree

1 file changed

+37
-19
lines changed

1 file changed

+37
-19
lines changed

crates/oxc_allocator/src/vec2/raw_vec.rs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -351,22 +351,6 @@ impl<'a, T> RawVec<'a, T> {
351351
}
352352
}
353353

354-
/// Calculates the buffer's new size given that it'll hold `len +
355-
/// additional` elements. This logic is used in amortized reserve methods.
356-
/// Returns `(new_capacity, new_alloc_size)`.
357-
fn amortized_new_size(
358-
&self,
359-
len: usize,
360-
additional: usize,
361-
) -> Result<usize, CollectionAllocErr> {
362-
// Nothing we can really do about these checks :(
363-
let required_cap = len.checked_add(additional).ok_or(CapacityOverflow)?;
364-
// Cannot overflow, because `cap <= isize::MAX`, and type of `cap` is `usize`.
365-
let double_cap = self.cap * 2;
366-
// `double_cap` guarantees exponential growth.
367-
Ok(cmp::max(double_cap, required_cap))
368-
}
369-
370354
/// The same as `reserve`, but returns on errors instead of panicking or aborting.
371355
pub fn try_reserve(&mut self, len: usize, additional: usize) -> Result<(), CollectionAllocErr> {
372356
if self.needs_to_grow(len, additional) {
@@ -635,12 +619,46 @@ impl<'a, T> RawVec<'a, T> {
635619
// If we make it past the first branch then we are guaranteed to
636620
// panic.
637621

638-
let new_cap = self.amortized_new_size(len, additional)?;
639-
let new_layout = Layout::array::<T>(new_cap).map_err(|_| CapacityOverflow)?;
622+
// Nothing we can really do about these checks, sadly.
623+
let required_cap = len.checked_add(additional).ok_or(CapacityOverflow)?;
624+
625+
// This guarantees exponential growth. The doubling cannot overflow
626+
// because `cap <= isize::MAX` and the type of `cap` is `usize`.
627+
let cap = cmp::max(self.cap() * 2, required_cap);
628+
629+
// The following commented-out code is copied from the standard library.
630+
// We don't use it because this would cause notable performance regression
631+
// in the oxc_transformer, oxc_minifier and oxc_mangler, but would only get a
632+
// tiny performance improvement in the oxc_parser.
633+
//
634+
// The reason is that only the oxc_parser has a lot of tiny `Vec`s without
635+
// pre-reserved capacity, which can benefit from this change. Other
636+
// crates don't have such cases, so the `cmp::max` calculation costs more
637+
// than the potential performance improvement.
638+
//
639+
// ------------------ Copied from the standard library ------------------
640+
//
641+
// Tiny Vecs are dumb. Skip to:
642+
// - 8 if the element size is 1, because any heap allocators is likely
643+
// to round up a request of less than 8 bytes to at least 8 bytes.
644+
// - 4 if elements are moderate-sized (<= 1 KiB).
645+
// - 1 otherwise, to avoid wasting too much space for very short Vecs.
646+
// const fn min_non_zero_cap(size: usize) -> usize {
647+
// if size == 1 {
648+
// 8
649+
// } else if size <= 1024 {
650+
// 4
651+
// } else {
652+
// 1
653+
// }
654+
// }
655+
// let cap = cmp::max(Self::MIN_NON_ZERO_CAP, cap);
656+
657+
let new_layout = Layout::array::<T>(cap).map_err(|_| CapacityOverflow)?;
640658

641659
self.ptr = self.finish_grow(new_layout)?.cast();
642660

643-
self.cap = new_cap;
661+
self.cap = cap;
644662

645663
Ok(())
646664
}

0 commit comments

Comments
 (0)