You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
AstNodeId, ScopeId, SymbolId and ReferenceId are used as indexes on IndexVecs. All are wrappers around NonMaxU32. NonMaxU32::from_usize bounds checks self.len() and panics if it's >= u32::MAX.
IndexVec::push is used extensively, so this extra panicking branch:
Is on a hot path.
Increases the size of push function, which makes it less likely to be inlined.
Vec::push already checks len vs capacity, and has a cold branch where the Vec needs to grow.
We can remove the extra bounds check in IndexVec::push by moving the bounds check into this cold path. As follows:
1. Extend Idx trait
traitIdx{constMAX:usize;unsafefnfrom_usize_unchecked(idx:usize) -> Self;fnindex(self) -> usize;fnfrom_usize(idx:usize) -> Self{assert!(idx <= Self::MAX);// SAFETY: We checked `idx` is validunsafe{Self::from_usize_unchecked(idx)}}}
impl<I:Idx,T>IndexVec<I,T>{// `std::vec::Vec`'s minimumconstMIN_CAPACITY:usize = match std::mem::size_of::<T>(){1 => 8,
s if s <= 1024 => 4,
_ => 1,};// Smaller of `I::MAX` items or `isize::MAX` bytesconstMAX_CAPACITY:usize = min(I::MAX,
isize::MAXasusize / max(std::mem::size_of::<T>(),1),);#[inline]pubfnpush(&mutself,value:T) -> I{let len = self.raw.len();// This branch is rarely takenif len == self.raw.capacity(){self.grow_one();}// Manually add `value` to the `Vec` (code copied from `std::vec::Vec`)unsafe{let end = self.raw.as_mut_ptr().add(len);
ptr::write(end, value);self.raw.set_len(len + 1);}// SAFETY: `len` cannot exceed `capacity`, and `capacity` is always `<= I::MAX`unsafe{I::from_usize_unchecked(len)}}#[cold]#[inline(never)]fngrow_one(&mutself){let current_capacity = self.raw.capacity();let new_capacity = (current_capacity *2).max(Self::MIN_CAPACITY).min(Self::MAX_CAPACITY);let additional = new_capacity - current_capacity;// Bounds check moved here, in cold branchassert!(additional > 0);// Avoid `reserve_exact()` doing bounds checks againlet len = self.raw.len();unsafe{assert_unchecked!(len == current_capacity);assert_unchecked!(len.checked_add(additional).is_some());}self.raw.reserve_exact(additional);}}constfnmin(n1:usize,n2:usize) -> usize{if n1 < n2 { n1 }else{ n2 }}constfnmax(n1:usize,n2:usize) -> usize{if n1 > n2 { n1 }else{ n2 }}
3. Prevent capacity ever exceeding I::MAX
Amend all other methods to ensure that capacity can never exceed MAX_CAPACITY.
Note: It's capacity which must not exceed MAX_CAPACITY, not len. This allows if len == self.0.capacity() in push to do double-duty of checking if Vec needs to grow, and ensuring indexes are in bounds, with a single check.
The text was updated successfully, but these errors were encountered:
If we restrict capacity (and therefore also len) to max I::MAX - 1, then we can also remove the bounds check from IndexVec::next_idx, which can then use I::from_usize_unchecked instead of I::from_usize.
oxc_index::IndexVec
contains the following implementation ofpush
:AstNodeId
,ScopeId
,SymbolId
andReferenceId
are used as indexes onIndexVec
s. All are wrappers aroundNonMaxU32
.NonMaxU32::from_usize
bounds checksself.len()
and panics if it's>= u32::MAX
.IndexVec::push
is used extensively, so this extra panicking branch:push
function, which makes it less likely to be inlined.Vec::push
already checkslen
vscapacity
, and has a cold branch where theVec
needs to grow.We can remove the extra bounds check in
IndexVec::push
by moving the bounds check into this cold path. As follows:1. Extend
Idx
trait2. Reimplement
push
3. Prevent
capacity
ever exceedingI::MAX
Amend all other methods to ensure that
capacity
can never exceedMAX_CAPACITY
.Note: It's
capacity
which must not exceedMAX_CAPACITY
, notlen
. This allowsif len == self.0.capacity()
inpush
to do double-duty of checking ifVec
needs to grow, and ensuring indexes are in bounds, with a single check.The text was updated successfully, but these errors were encountered: