From 75d0c6aa867767a52604b535d99db948243b19fe Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:56:08 +0100 Subject: [PATCH 01/11] implement UniqueEntityVec --- crates/bevy_ecs/src/entity/mod.rs | 4 + crates/bevy_ecs/src/entity/unique_vec.rs | 699 +++++++++++++++++++++++ 2 files changed, 703 insertions(+) create mode 100644 crates/bevy_ecs/src/entity/unique_vec.rs diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 3fa63d37fa777..8be4df3643149 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -50,6 +50,10 @@ pub use entity_set::*; pub use map_entities::*; pub use visit_entities::*; +mod unique_vec; + +pub use unique_vec::*; + mod hash; pub use hash::*; diff --git a/crates/bevy_ecs/src/entity/unique_vec.rs b/crates/bevy_ecs/src/entity/unique_vec.rs new file mode 100644 index 0000000000000..cdb3959dc3e4c --- /dev/null +++ b/crates/bevy_ecs/src/entity/unique_vec.rs @@ -0,0 +1,699 @@ +use core::{ + borrow::Borrow, + mem::MaybeUninit, + ops::{Index, RangeBounds}, + slice, +}; + +use alloc::{ + borrow::Cow, + boxed::Box, + collections::{BTreeSet, BinaryHeap, TryReserveError, VecDeque}, + rc::Rc, + sync::Arc, + vec::{self, Vec}, +}; + +use super::{EntitySet, FromEntitySetIterator, TrustedEntityBorrow, UniqueEntityIter}; + +/// A `Vec` that contains only unique entities. +/// +/// "Unique" means that `x != y` holds for any 2 entities in this collection. +/// This is always true when less than 2 entities are present. +/// +/// This type is best obtained by its `FromEntitySetIterator` impl, via either +/// `EntityIterator::collect_set` or `UniqueEntityVec::from_entity_iter`. +/// +/// While this type can be constructed via `Iterator::collect`, doing so is inefficient, +/// and not recommended. +#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct UniqueEntityVec(Vec); + +impl UniqueEntityVec { + /// Constructs a new, empty `UniqueEntityVec`. + /// + /// Equivalent to [`Vec::new`]. + pub const fn new() -> Self { + Self(Vec::new()) + } + + /// Constructs a new, empty `UniqueEntityVec` with at least the specified capacity. + /// + /// Equivalent to [`Vec::with_capacity`] + pub fn with_capacity(capacity: usize) -> Self { + Self(Vec::with_capacity(capacity)) + } + + /// Creates a `UniqueEntityVec` directly from a pointer, a length, and a capacity. + /// + /// Equivalent to [`Vec::from_raw_parts`]. + /// + /// # Safety + /// + /// It must be safe to call [`Vec::from_raw_parts`] with these inputs, + /// and the resulting [`Vec`] must only contain unique elements. + pub unsafe fn from_raw_parts(ptr: *mut T, length: usize, capacity: usize) -> Self { + Self(Vec::from_raw_parts(ptr, length, capacity)) + } + + /// Constructs a `UniqueEntityVec` from a [`Vec`] unsafely. + /// + /// # Safety + /// + /// `vec` must contain only unique elements. + pub unsafe fn from_vec_unchecked(vec: Vec) -> Self { + Self(vec) + } + + /// Returns the inner [`Vec`]. + pub fn into_inner(self) -> Vec { + self.0 + } + + /// Returns a reference to the inner [`Vec`]. + pub fn as_vec(&self) -> &Vec { + &self.0 + } + + /// Returns a mutable reference to the inner [`Vec`]. + /// + /// # Safety + /// + /// The elements of this `Vec` must always remain unique, even while + /// this mutable reference is live. + pub unsafe fn as_mut_vec(&mut self) -> &mut Vec { + &mut self.0 + } + + /// Returns the total number of elements the vector can hold without + /// reallocating. + /// + /// Equivalent to [`Vec::capacity`]. + pub fn capacity(&self) -> usize { + self.0.capacity() + } + + /// Reserves capacity for at least `additional` more elements to be inserted + /// in the given `Vec`. + /// + /// Equivalent to [`Vec::reserve`]. + pub fn reserve(&mut self, additional: usize) { + self.0.reserve(additional); + } + + /// Reserves the minimum capacity for at least `additional` more elements to + /// be inserted in the given `UniqueEntityVec`. + /// + /// Equivalent to [`Vec::reserve_exact`]. + pub fn reserve_exact(&mut self, additional: usize) { + self.0.reserve_exact(additional); + } + + /// Tries to reserve capacity for at least `additional` more elements to be inserted + /// in the given `Vec`. + /// + /// Equivalent to [`Vec::try_reserve`]. + pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.0.try_reserve(additional) + } + + /// Tries to reserve the minimum capacity for at least `additional` + /// elements to be inserted in the given `Vec`. + /// + /// Equivalent to [`Vec::try_reserve_exact`]. + pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> { + self.0.try_reserve_exact(additional) + } + + /// Shrinks the capacity of the vector as much as possible. + /// + /// Equivalent to [`Vec::shrink_to_fit`]. + pub fn shrink_to_fit(&mut self) { + self.0.shrink_to_fit(); + } + + /// Shrinks the capacity of the vector with a lower bound. + /// + /// Equivalent to [`Vec::shrink_to`]. + pub fn shrink_to(&mut self, min_capacity: usize) { + self.0.shrink_to(min_capacity); + } + + /// Shortens the vector, keeping the first `len` elements and dropping + /// the rest. + /// + /// Equivalent to [`Vec::truncate`]. + pub fn truncate(&mut self, len: usize) { + self.0.truncate(len); + } + + /// Returns a raw pointer to the vector's buffer, or a dangling raw pointer + /// valid for zero sized reads if the vector didn't allocate. + /// + /// Equivalent to [`Vec::as_ptr`]. + pub fn as_ptr(&self) -> *const T { + self.0.as_ptr() + } + /// Returns a raw mutable pointer to the vector's buffer, or a dangling + /// raw pointer valid for zero sized reads if the vector didn't allocate. + /// + /// Equivalent to [`Vec::as_mut_ptr`]. + pub fn as_mut_ptr(&mut self) -> *mut T { + self.0.as_mut_ptr() + } + + /// Forces the length of the vector to `new_len`. + /// + /// Equivalent to [`Vec::set_len`]. + /// + /// # Safety + /// + /// It must be safe to call [`Vec::set_len`] with these inputs, + /// and the resulting [`Vec`] must only contain unique elements. + pub unsafe fn set_len(&mut self, new_len: usize) { + self.0.set_len(new_len); + } + + /// Removes an element from the vector and returns it. + /// + /// Equivalent to [`Vec::swap_remove`]. + pub fn swap_remove(&mut self, index: usize) -> T { + self.0.swap_remove(index) + } + + /// Inserts an element at position `index` within the vector, shifting all + /// elements after it to the right. + /// + /// Equivalent to [`Vec::insert`]. + /// + /// # Safety + /// + /// No `T` contained by `self` may equal `element`. + pub unsafe fn insert(&mut self, index: usize, element: T) { + self.0.insert(index, element); + } + + /// Retains only the elements specified by the predicate. + /// + /// Equivalent to [`Vec::retain`]. + pub fn retain(&mut self, f: F) + where + F: FnMut(&T) -> bool, + { + self.0.retain(f); + } + + /// Retains only the elements specified by the predicate, passing a mutable reference to it. + /// + /// Equivalent to [`Vec::retain_mut`]. + /// + /// # Safety + /// + /// `self` must only contain unique elements after each individual execution of `f`. + pub unsafe fn retain_mut(&mut self, f: F) + where + F: FnMut(&mut T) -> bool, + { + self.0.retain_mut(f); + } + + /// Removes all but the first of consecutive elements in the vector that resolve to the same + /// key. + /// + /// Equivalent to [`Vec::dedup_by_key`]. + /// + /// # Safety + /// + /// `self` must only contain unique elements after each individual execution of `key`. + pub unsafe fn dedup_by_key(&mut self, key: F) + where + F: FnMut(&mut T) -> K, + K: PartialEq, + { + self.0.dedup_by_key(key); + } + + /// Removes all but the first of consecutive elements in the vector satisfying a given equality + /// relation. + /// + /// Equivalent to [`Vec::dedup_by`]. + /// + /// # Safety + /// + /// `self` must only contain unique elements after each individual execution of `same_bucket`. + pub unsafe fn dedup_by(&mut self, same_bucket: F) + where + F: FnMut(&mut T, &mut T) -> bool, + { + self.0.dedup_by(same_bucket); + } + + /// Appends an element to the back of a collection. + /// + /// Equivalent to [`Vec::push`]. + /// + /// # Safety + /// + /// No `T` contained by `self` may equal `element`. + pub unsafe fn push(&mut self, value: T) { + self.0.push(value); + } + + /// Moves all the elements of `other` into `self`, leaving `other` empty. + /// + /// Equivalent to [`Vec::append`]. + /// + /// # Safety + /// + /// `other` must contain no elements that equal any element in `self`. + pub unsafe fn append(&mut self, other: &mut UniqueEntityVec) { + self.0.append(&mut other.0); + } + + /// Removes the last element from a vector and returns it, or [`None`] if it + /// is empty. + /// + /// Equivalent to [`Vec::pop`]. + pub fn pop(&mut self) -> Option { + self.0.pop() + } + + /// Removes the specified range from the vector in bulk, returning all + /// removed elements as an iterator. + /// + /// Equivalent to [`Vec::drain`]. + pub fn drain(&mut self, range: R) -> Drain<'_, T> + where + R: RangeBounds, + { + // SAFETY: `self` and thus `range` contains only unique elements. + unsafe { UniqueEntityIter::from_iterator_unchecked(self.0.drain(range)) } + } + + /// Clears the vector, removing all values. + /// + /// Equivalent to [`Vec::clear`]. + pub fn clear(&mut self) { + self.0.clear(); + } + + /// Returns the number of elements in the vector, also referred to + /// as its 'length'. + /// + /// Equivalent to [`Vec::len`]. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns `true` if the vector contains no elements. + /// + /// Equivalent to [`Vec::is_empty`]. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Splits the collection into two at the given index. + /// + /// Equivalent to [`Vec::split_off`]. + pub fn split_off(&mut self, at: usize) -> Self { + Self(self.0.split_off(at)) + } + + /// Resizes the `Vec` in-place so that `len` is equal to `new_len`. + /// + /// Equivalent to [`Vec::resize_with`]. + /// + /// # Safety + /// + /// `f` must only produce unique `T`, and none of these may equal any `T` in `self`. + pub unsafe fn resize_with(&mut self, new_len: usize, f: F) + where + F: FnMut() -> T, + { + self.0.resize_with(new_len, f); + } + + /// Returns the remaining spare capacity of the vector as a slice of + /// [`MaybeUninit`]. + /// + /// Equivalent to [`Vec::spare_capacity_mut`]. + pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit] { + self.0.spare_capacity_mut() + } + + /// Creates a splicing iterator that replaces the specified range in the vector + /// with the given `replace_with` iterator and yields the removed items. + /// + /// Equivalent to [`Vec::splice`]. + /// + /// # Safety + /// + /// `replace_with` must not yield any elements that equal any elements in `self`, + /// except for those in `range`. + pub unsafe fn splice( + &mut self, + range: R, + replace_with: I, + ) -> Splice<'_, ::IntoIter> + where + R: RangeBounds, + I: EntitySet, + { + // SAFETY: `self` and thus `range` contains only unique elements. + UniqueEntityIter::from_iterator_unchecked(self.0.splice(range, replace_with)) + } +} + +impl Default for UniqueEntityVec { + fn default() -> Self { + Self(Vec::default()) + } +} + +impl<'a, T: TrustedEntityBorrow> IntoIterator for &'a UniqueEntityVec +where + &'a T: TrustedEntityBorrow, +{ + type Item = &'a T; + + type IntoIter = UniqueEntityIter>; + + fn into_iter(self) -> Self::IntoIter { + // SAFETY: `self` contains only unique elements. + unsafe { UniqueEntityIter::from_iterator_unchecked(self.0.iter()) } + } +} + +impl IntoIterator for UniqueEntityVec { + type Item = T; + + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + // SAFETY: `self` contains only unique elements. + unsafe { UniqueEntityIter::from_iterator_unchecked(self.0.into_iter()) } + } +} + +impl AsMut for UniqueEntityVec { + fn as_mut(&mut self) -> &mut UniqueEntityVec { + self + } +} + +impl AsRef for UniqueEntityVec { + fn as_ref(&self) -> &Self { + self + } +} + +impl AsRef> for UniqueEntityVec { + fn as_ref(&self) -> &Vec { + &self.0 + } +} + +impl Borrow> for UniqueEntityVec { + fn borrow(&self) -> &Vec { + &self.0 + } +} + +impl AsRef<[T]> for UniqueEntityVec { + fn as_ref(&self) -> &[T] { + &self.0 + } +} + +impl Borrow<[T]> for UniqueEntityVec { + fn borrow(&self) -> &[T] { + &self.0 + } +} + +impl, U> PartialEq> for UniqueEntityVec { + fn eq(&self, other: &Vec) -> bool { + self.0.eq(other) + } +} + +impl, U> PartialEq<&[U]> for UniqueEntityVec { + fn eq(&self, other: &&[U]) -> bool { + self.0.eq(other) + } +} + +impl, U> PartialEq<&mut [U]> for UniqueEntityVec { + fn eq(&self, other: &&mut [U]) -> bool { + self.0.eq(other) + } +} + +impl, U, const N: usize> PartialEq<&[U; N]> + for UniqueEntityVec +{ + fn eq(&self, other: &&[U; N]) -> bool { + self.0.eq(other) + } +} + +impl, U, const N: usize> PartialEq<&mut [U; N]> + for UniqueEntityVec +{ + fn eq(&self, other: &&mut [U; N]) -> bool { + self.0.eq(&**other) + } +} + +impl, U> PartialEq<[U]> for UniqueEntityVec { + fn eq(&self, other: &[U]) -> bool { + self.0.eq(other) + } +} + +impl, U, const N: usize> PartialEq<[U; N]> + for UniqueEntityVec +{ + fn eq(&self, other: &[U; N]) -> bool { + self.0.eq(other) + } +} + +impl, U: TrustedEntityBorrow> PartialEq> for Vec { + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl, U: TrustedEntityBorrow> PartialEq> for &[T] { + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl, U: TrustedEntityBorrow> PartialEq> for &mut [T] { + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl, U: TrustedEntityBorrow> PartialEq> + for [T] +{ + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl + Clone, U: TrustedEntityBorrow> PartialEq> + for Cow<'_, [T]> +{ + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl, U: TrustedEntityBorrow> PartialEq> for VecDeque { + fn eq(&self, other: &UniqueEntityVec) -> bool { + self.eq(&other.0) + } +} + +impl From<&[T; 1]> for UniqueEntityVec { + fn from(value: &[T; 1]) -> Self { + Self(Vec::from(value)) + } +} + +impl From<&[T; 0]> for UniqueEntityVec { + fn from(value: &[T; 0]) -> Self { + Self(Vec::from(value)) + } +} + +impl From<&mut [T; 1]> for UniqueEntityVec { + fn from(value: &mut [T; 1]) -> Self { + Self(Vec::from(value)) + } +} + +impl From<&mut [T; 0]> for UniqueEntityVec { + fn from(value: &mut [T; 0]) -> Self { + Self(Vec::from(value)) + } +} + +impl From<[T; 1]> for UniqueEntityVec { + fn from(value: [T; 1]) -> Self { + Self(Vec::from(value)) + } +} + +impl From<[T; 0]> for UniqueEntityVec { + fn from(value: [T; 0]) -> Self { + Self(Vec::from(value)) + } +} + +impl From> for Vec { + fn from(value: UniqueEntityVec) -> Self { + value.0 + } +} + +impl<'a, T: TrustedEntityBorrow + Clone> From> for Cow<'a, [T]> { + fn from(value: UniqueEntityVec) -> Self { + Cow::from(value.0) + } +} + +impl From> for Arc<[T]> { + fn from(value: UniqueEntityVec) -> Self { + Arc::from(value.0) + } +} + +impl From> for BinaryHeap { + fn from(value: UniqueEntityVec) -> Self { + BinaryHeap::from(value.0) + } +} + +impl From> for Box<[T]> { + fn from(value: UniqueEntityVec) -> Self { + Box::from(value.0) + } +} + +impl From> for Rc<[T]> { + fn from(value: UniqueEntityVec) -> Self { + Rc::from(value.0) + } +} + +impl From> for VecDeque { + fn from(value: UniqueEntityVec) -> Self { + VecDeque::from(value.0) + } +} + +impl TryFrom> for Box<[T; N]> { + type Error = UniqueEntityVec; + + fn try_from(value: UniqueEntityVec) -> Result { + Box::try_from(value.0).map_err(UniqueEntityVec) + } +} + +impl TryFrom> for [T; N] { + type Error = UniqueEntityVec; + + fn try_from(value: UniqueEntityVec) -> Result { + <[T; N] as TryFrom>>::try_from(value.0).map_err(UniqueEntityVec) + } +} + +impl From> for UniqueEntityVec { + fn from(value: BTreeSet) -> Self { + Self(value.into_iter().collect::>()) + } +} + +impl FromIterator for UniqueEntityVec { + fn from_iter>(iter: I) -> Self { + let iter = iter.into_iter(); + let unique_vec = Self::with_capacity(iter.size_hint().0); + iter.fold(unique_vec, |mut unique_vec, item| { + if unique_vec.0.iter().all(|e| e.entity().ne(&item.entity())) { + unique_vec.0.push(item); + } + unique_vec + }) + } +} + +impl FromEntitySetIterator for UniqueEntityVec { + fn from_entity_set_iter>(iter: I) -> Self { + // SAFETY: `iter` is an `EntitySet`. + unsafe { Self::from_vec_unchecked(Vec::from_iter(iter)) } + } +} + +impl Extend for UniqueEntityVec { + fn extend>(&mut self, iter: I) { + let iter = iter.into_iter(); + let reserve = if self.is_empty() { + iter.size_hint().0 + } else { + (iter.size_hint().0 + 1) / 2 + }; + self.reserve(reserve); + iter.for_each(move |item| { + if self.0.iter().all(|e| e.entity().ne(&item.entity())) { + self.0.push(item); + } + }); + } +} + +impl<'a, T: TrustedEntityBorrow + Copy + 'a> Extend<&'a T> for UniqueEntityVec { + fn extend>(&mut self, iter: I) { + let iter = iter.into_iter(); + let reserve = if self.is_empty() { + iter.size_hint().0 + } else { + (iter.size_hint().0 + 1) / 2 + }; + self.reserve(reserve); + iter.for_each(move |item| { + if self.0.iter().all(|e| e.entity().ne(&item.entity())) { + self.0.push(*item); + } + }); + } +} + +impl Index for UniqueEntityVec { + type Output = T; + fn index(&self, key: usize) -> &T { + self.0.index(key) + } +} + +/// A draining iterator for [`UniqueEntityVec`]. +/// +/// This struct is created by [`UniqueEntityVec::drain`]. +/// See its documentation for more. +pub type Drain<'a, T> = UniqueEntityIter>; + +/// A splicing iterator for [`UniqueEntityVec`]. +/// +/// This struct is created by [`UniqueEntityVec::splice`]. +/// See its documentation for more. +pub type Splice<'a, I> = UniqueEntityIter>; + +/// An iterator that moves out of a vector. +/// +/// This `struct` is created by the [`IntoIterator::into_iter`] trait +/// method on [`UniqueEntityVec`]. +pub type IntoIter = UniqueEntityIter>; From aa36aa36817014813bd8e94dbb10c5e9ecd043fd Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 18:05:36 +0100 Subject: [PATCH 02/11] address comments --- crates/bevy_ecs/src/entity/unique_vec.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/entity/unique_vec.rs b/crates/bevy_ecs/src/entity/unique_vec.rs index cdb3959dc3e4c..33630335305bc 100644 --- a/crates/bevy_ecs/src/entity/unique_vec.rs +++ b/crates/bevy_ecs/src/entity/unique_vec.rs @@ -53,7 +53,8 @@ impl UniqueEntityVec { /// It must be safe to call [`Vec::from_raw_parts`] with these inputs, /// and the resulting [`Vec`] must only contain unique elements. pub unsafe fn from_raw_parts(ptr: *mut T, length: usize, capacity: usize) -> Self { - Self(Vec::from_raw_parts(ptr, length, capacity)) + // SAFETY: Caller ensures it's safe to call `Vec::from_raw_parts` + Self(unsafe { Vec::from_raw_parts(ptr, length, capacity) }) } /// Constructs a `UniqueEntityVec` from a [`Vec`] unsafely. @@ -171,7 +172,8 @@ impl UniqueEntityVec { /// It must be safe to call [`Vec::set_len`] with these inputs, /// and the resulting [`Vec`] must only contain unique elements. pub unsafe fn set_len(&mut self, new_len: usize) { - self.0.set_len(new_len); + // SAFETY: Caller ensures it's safe to call `Vec::set_len` + unsafe { self.0.set_len(new_len) }; } /// Removes an element from the vector and returns it. @@ -193,6 +195,14 @@ impl UniqueEntityVec { self.0.insert(index, element); } + /// Removes and returns the element at position `index` within the vector, + /// shifting all elements after it to the left. + /// + /// Equivalent to [`Vec::remove`]. + pub fn remove(&mut self, index: usize) -> T { + self.0.remove(index) + } + /// Retains only the elements specified by the predicate. /// /// Equivalent to [`Vec::retain`]. @@ -624,7 +634,7 @@ impl FromIterator for UniqueEntityVec { let iter = iter.into_iter(); let unique_vec = Self::with_capacity(iter.size_hint().0); iter.fold(unique_vec, |mut unique_vec, item| { - if unique_vec.0.iter().all(|e| e.entity().ne(&item.entity())) { + if !unique_vec.0.contains(&item) { unique_vec.0.push(item); } unique_vec @@ -649,7 +659,7 @@ impl Extend for UniqueEntityVec { }; self.reserve(reserve); iter.for_each(move |item| { - if self.0.iter().all(|e| e.entity().ne(&item.entity())) { + if !self.0.contains(&item) { self.0.push(item); } }); @@ -666,7 +676,7 @@ impl<'a, T: TrustedEntityBorrow + Copy + 'a> Extend<&'a T> for UniqueEntityVec Date: Mon, 27 Jan 2025 18:28:43 +0100 Subject: [PATCH 03/11] add further documentation on Extend and FromIterator impls --- crates/bevy_ecs/src/entity/unique_vec.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/bevy_ecs/src/entity/unique_vec.rs b/crates/bevy_ecs/src/entity/unique_vec.rs index 33630335305bc..59e57a200248d 100644 --- a/crates/bevy_ecs/src/entity/unique_vec.rs +++ b/crates/bevy_ecs/src/entity/unique_vec.rs @@ -630,6 +630,9 @@ impl From> for UniqueEntityVec { } impl FromIterator for UniqueEntityVec { + /// This impl only uses `Eq` to validate uniqueness, resulting in O(n^2) complexity. + /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. + /// When possible, use `FromEntitySetIterator::from_entity_iter` instead. fn from_iter>(iter: I) -> Self { let iter = iter.into_iter(); let unique_vec = Self::with_capacity(iter.size_hint().0); @@ -650,6 +653,9 @@ impl FromEntitySetIterator for UniqueEntityVec { } impl Extend for UniqueEntityVec { + /// Use with caution, because this impl only uses `Eq` to validate uniqueness, + /// resulting in O(n^2) complexity. + /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. fn extend>(&mut self, iter: I) { let iter = iter.into_iter(); let reserve = if self.is_empty() { @@ -667,6 +673,9 @@ impl Extend for UniqueEntityVec { } impl<'a, T: TrustedEntityBorrow + Copy + 'a> Extend<&'a T> for UniqueEntityVec { + /// Use with caution, because this impl only uses `Eq` to validate uniqueness, + /// resulting in O(n^2) complexity. + /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. fn extend>(&mut self, iter: I) { let iter = iter.into_iter(); let reserve = if self.is_empty() { From 2054fcefd247973486d4f6cbe7f7561d06dff4f4 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 18:35:59 +0100 Subject: [PATCH 04/11] Add further comments on reservation logic and fold/for_each usage --- crates/bevy_ecs/src/entity/unique_vec.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/unique_vec.rs b/crates/bevy_ecs/src/entity/unique_vec.rs index 59e57a200248d..32b4f0b03751f 100644 --- a/crates/bevy_ecs/src/entity/unique_vec.rs +++ b/crates/bevy_ecs/src/entity/unique_vec.rs @@ -634,8 +634,11 @@ impl FromIterator for UniqueEntityVec { /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. /// When possible, use `FromEntitySetIterator::from_entity_iter` instead. fn from_iter>(iter: I) -> Self { + // Matches the `HashSet::from_iter` reservation logic. let iter = iter.into_iter(); let unique_vec = Self::with_capacity(iter.size_hint().0); + // Internal iteration (fold/for_each) is known to result in better code generation + // over a for loop. iter.fold(unique_vec, |mut unique_vec, item| { if !unique_vec.0.contains(&item) { unique_vec.0.push(item); @@ -653,10 +656,15 @@ impl FromEntitySetIterator for UniqueEntityVec { } impl Extend for UniqueEntityVec { - /// Use with caution, because this impl only uses `Eq` to validate uniqueness, + /// Use with caution, because this impl only uses `Eq` to validate uniqueness, /// resulting in O(n^2) complexity. /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. fn extend>(&mut self, iter: I) { + // Matches the `HashSet::extend` reservation logic. Their reasoning: + // "Keys may be already present or show multiple times in the iterator. + // Reserve the entire hint lower bound if the map is empty. + // Otherwise reserve half the hint (rounded up), so the map + // will only resize twice in the worst case." let iter = iter.into_iter(); let reserve = if self.is_empty() { iter.size_hint().0 @@ -664,6 +672,8 @@ impl Extend for UniqueEntityVec { (iter.size_hint().0 + 1) / 2 }; self.reserve(reserve); + // Internal iteration (fold/for_each) is known to result in better code generation + // over a for loop. iter.for_each(move |item| { if !self.0.contains(&item) { self.0.push(item); @@ -673,10 +683,15 @@ impl Extend for UniqueEntityVec { } impl<'a, T: TrustedEntityBorrow + Copy + 'a> Extend<&'a T> for UniqueEntityVec { - /// Use with caution, because this impl only uses `Eq` to validate uniqueness, + /// Use with caution, because this impl only uses `Eq` to validate uniqueness, /// resulting in O(n^2) complexity. /// It can make sense for very low N, or if `T` implements neither `Ord` nor `Hash`. fn extend>(&mut self, iter: I) { + // Matches the `HashSet::extend` reservation logic. Their reasoning: + // "Keys may be already present or show multiple times in the iterator. + // Reserve the entire hint lower bound if the map is empty. + // Otherwise reserve half the hint (rounded up), so the map + // will only resize twice in the worst case." let iter = iter.into_iter(); let reserve = if self.is_empty() { iter.size_hint().0 @@ -684,6 +699,8 @@ impl<'a, T: TrustedEntityBorrow + Copy + 'a> Extend<&'a T> for UniqueEntityVec Date: Mon, 27 Jan 2025 19:43:04 +0100 Subject: [PATCH 05/11] Add UniqueEntityVec::collect to disallowed methods --- crates/bevy_ecs/clippy.toml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 crates/bevy_ecs/clippy.toml diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml new file mode 100644 index 0000000000000..40cec4d87bff6 --- /dev/null +++ b/crates/bevy_ecs/clippy.toml @@ -0,0 +1,3 @@ +disallowed-methods = [ + { path = "UniqueEntityVec::collect", reason = "Use UniqueEntityVec::collect_set if possible" }, + ] \ No newline at end of file From 05efc3e59d4b0d2de0b6476aefe2118bdacaa0f3 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 19:45:44 +0100 Subject: [PATCH 06/11] correct the disallowed method --- crates/bevy_ecs/clippy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index 40cec4d87bff6..594b6a5c40841 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,3 +1,3 @@ disallowed-methods = [ - { path = "UniqueEntityVec::collect", reason = "Use UniqueEntityVec::collect_set if possible" }, + { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, ] \ No newline at end of file From d4a849cf2e056e0a2516fb9e2fe47837c9c17944 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 19:52:17 +0100 Subject: [PATCH 07/11] clippy toml formatting --- crates/bevy_ecs/clippy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index 594b6a5c40841..67800565289f3 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,3 +1,3 @@ disallowed-methods = [ - { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, + { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, ] \ No newline at end of file From 3474bd5abda37982ee014ba4e3b7c94a51adda6c Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 20:00:25 +0100 Subject: [PATCH 08/11] format clippy toml correctly --- crates/bevy_ecs/clippy.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index 67800565289f3..e3fac67909b9d 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,3 +1,3 @@ disallowed-methods = [ - { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, - ] \ No newline at end of file + { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, +] \ No newline at end of file From e9801a75e4404e5b88eba31694ff235bff55c3c2 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 20:06:54 +0100 Subject: [PATCH 09/11] add EntitySetIterator::collect:: to disallowed methods, clarify reason --- crates/bevy_ecs/clippy.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index e3fac67909b9d..35758ccd584af 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,3 +1,4 @@ disallowed-methods = [ - { path = "UniqueEntityVec::from_iter", reason = "Use EntitySetIterator::collect_set or UniqueEntityVec::from_entity_set_iter if possible" }, + { path = "UniqueEntityVec::from_iter", reason = "Use UniqueEntityVec::from_entity_set_iter if possible, it skips validation" }, + { path = "EntitySetIterator::collect::", reason = "Use EntitySetIterator::collect_set if possible, it skips validation" }, ] \ No newline at end of file From f8bd259cc0507491d9e62ad328c2120d40c669b9 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 20:12:41 +0100 Subject: [PATCH 10/11] format clippy toml once more --- crates/bevy_ecs/clippy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index 35758ccd584af..3e968f2d614fb 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,4 +1,4 @@ -disallowed-methods = [ +disallowed-methods = [ { path = "UniqueEntityVec::from_iter", reason = "Use UniqueEntityVec::from_entity_set_iter if possible, it skips validation" }, { path = "EntitySetIterator::collect::", reason = "Use EntitySetIterator::collect_set if possible, it skips validation" }, ] \ No newline at end of file From e36321e5a4830645d63fae6352039270abdfa1d7 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Mon, 27 Jan 2025 20:25:19 +0100 Subject: [PATCH 11/11] add a newline at the end of clippy-toml --- crates/bevy_ecs/clippy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/clippy.toml b/crates/bevy_ecs/clippy.toml index 3e968f2d614fb..6532f723e5e2e 100644 --- a/crates/bevy_ecs/clippy.toml +++ b/crates/bevy_ecs/clippy.toml @@ -1,4 +1,4 @@ disallowed-methods = [ { path = "UniqueEntityVec::from_iter", reason = "Use UniqueEntityVec::from_entity_set_iter if possible, it skips validation" }, { path = "EntitySetIterator::collect::", reason = "Use EntitySetIterator::collect_set if possible, it skips validation" }, -] \ No newline at end of file +]