From 3e48e50b8240f41630c14f383aeeb5d078d1dbbf Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Thu, 23 Jan 2025 09:07:34 +0100 Subject: [PATCH 1/4] implement FromEntitySetIterator --- crates/bevy_ecs/src/entity/entity_set.rs | 45 +++++++++++++++++++++++- crates/bevy_ecs/src/entity/hash_set.rs | 13 ++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/entity_set.rs b/crates/bevy_ecs/src/entity/entity_set.rs index 8b1f47779ef86..e78d087d7e090 100644 --- a/crates/bevy_ecs/src/entity/entity_set.rs +++ b/crates/bevy_ecs/src/entity/entity_set.rs @@ -3,10 +3,12 @@ use alloc::{ collections::{btree_map, btree_set}, rc::Rc, }; +use bevy_utils::hashbrown::HashSet; use core::{ array, fmt::{Debug, Formatter}, + hash::{BuildHasher, Hash}, iter::{self, FusedIterator}, option, result, }; @@ -144,7 +146,21 @@ impl> EntitySet for T {} /// /// `x != y` must hold for any 2 elements returned by the iterator. /// This is always true for iterators that cannot return more than one element. -pub unsafe trait EntitySetIterator: Iterator {} +pub unsafe trait EntitySetIterator: Iterator { + /// Transforms an `EntitySetIterator` into a collection. + /// + /// This is a specialized form of [`collect`], for collections which benefit from the uniqueness guarantee. + /// When present, this should always be preferred over [`collect`]. + /// + /// [`collect`]: Iterator::collect + // FIXME: When subtrait item shadowing stabilizes, this should be renamed and shadow `Iterator::collect` + fn collect_set>(self) -> B + where + Self: Sized, + { + FromEntitySetIterator::from_entity_set_iter(self) + } +} // SAFETY: // A correct `BTreeMap` contains only unique keys. @@ -291,6 +307,33 @@ unsafe impl::Item) -> bool> Enti // SAFETY: Discarding elements maintains uniqueness. unsafe impl EntitySetIterator for iter::StepBy {} +/// Conversion from an `EntitySetIterator`. +/// +/// Some collections, while they can be constructed from plain iterators, +/// benefit strongly from the additional uniqeness guarantee [`EntitySetIterator`] offers. +/// Mirroring [`Iterator::collect`]/[`FromIterator::from_iter`], [`EntitySetIterator::collect_set`] and +/// `FromEntitySetIterator::from_entity_set_iter` can be used for construction. +/// +/// See also: [`EntitySet`]. +// FIXME: When subtrait item shadowing stabilizes, this should be renamed and shadow `FromIterator::from_iter` +pub trait FromEntitySetIterator: FromIterator { + /// Creates a value from an [`EntitySetIterator`]. + fn from_entity_set_iter>(set_iter: T) -> Self; +} + +impl FromEntitySetIterator + for HashSet +{ + fn from_entity_set_iter>(set_iter: I) -> Self { + let mut set = HashSet::::default(); + for e in set_iter { + // SAFETY: Every element in self is unique. + unsafe { set.insert_unique_unchecked(e) }; + } + set + } +} + /// An iterator that yields unique entities. /// /// This wrapper can provide an [`EntitySetIterator`] implementation when an instance of `I` is known to uphold uniqueness. diff --git a/crates/bevy_ecs/src/entity/hash_set.rs b/crates/bevy_ecs/src/entity/hash_set.rs index d1b917388ea33..e9e7cde5d6d80 100644 --- a/crates/bevy_ecs/src/entity/hash_set.rs +++ b/crates/bevy_ecs/src/entity/hash_set.rs @@ -16,7 +16,7 @@ use core::{ use bevy_reflect::Reflect; use bevy_utils::hashbrown::hash_set::{self, HashSet}; -use super::{Entity, EntityHash, EntitySetIterator}; +use super::{Entity, EntityHash, EntitySet, EntitySetIterator, FromEntitySetIterator}; /// A [`HashSet`] pre-configured to use [`EntityHash`] hashing. #[cfg_attr(feature = "bevy_reflect", derive(Reflect))] @@ -195,6 +195,17 @@ impl FromIterator for EntityHashSet { } } +impl FromEntitySetIterator for EntityHashSet { + fn from_entity_set_iter>(set_iter: I) -> Self { + let mut set = EntityHashSet::new(); + for e in set_iter { + // SAFETY: Every element in self is unique. + unsafe { set.insert_unique_unchecked(e) }; + } + set + } +} + /// An iterator over the items of an [`EntityHashSet`]. /// /// This struct is created by the [`iter`] method on [`EntityHashSet`]. See its documentation for more. From 35cc4baa5ba04929228daf3005977773f447b072 Mon Sep 17 00:00:00 2001 From: Vic <59878206+Victoronz@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:01:24 +0100 Subject: [PATCH 2/4] Apply suggestions from code review Use size hint in the HashSet FromEntitySetIterator impls Co-authored-by: SpecificProtagonist --- crates/bevy_ecs/src/entity/entity_set.rs | 5 +++-- crates/bevy_ecs/src/entity/hash_set.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/entity/entity_set.rs b/crates/bevy_ecs/src/entity/entity_set.rs index e78d087d7e090..5202c6d9f82a5 100644 --- a/crates/bevy_ecs/src/entity/entity_set.rs +++ b/crates/bevy_ecs/src/entity/entity_set.rs @@ -325,8 +325,9 @@ impl FromEntitySetItera for HashSet { fn from_entity_set_iter>(set_iter: I) -> Self { - let mut set = HashSet::::default(); - for e in set_iter { + let iter = set_iter.into_iter(); + let mut set = HashSet::::with_capacity_and_hasher(iter.size_hint().0, S::default()); + for e in iter { // SAFETY: Every element in self is unique. unsafe { set.insert_unique_unchecked(e) }; } diff --git a/crates/bevy_ecs/src/entity/hash_set.rs b/crates/bevy_ecs/src/entity/hash_set.rs index e9e7cde5d6d80..dd368ff671014 100644 --- a/crates/bevy_ecs/src/entity/hash_set.rs +++ b/crates/bevy_ecs/src/entity/hash_set.rs @@ -197,8 +197,9 @@ impl FromIterator for EntityHashSet { impl FromEntitySetIterator for EntityHashSet { fn from_entity_set_iter>(set_iter: I) -> Self { - let mut set = EntityHashSet::new(); - for e in set_iter { + let iter = set_iter.into_iter(); + let mut set = EntityHashSet::with_capacity(iter.size_hint().0); + for e in iter { // SAFETY: Every element in self is unique. unsafe { set.insert_unique_unchecked(e) }; } From 939140fe33cb1d6578e77d5427dd3c5fc626c3fe Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:11:32 +0100 Subject: [PATCH 3/4] use bevy_platform_support --- crates/bevy_ecs/src/entity/entity_set.rs | 2 +- crates/bevy_ecs/src/entity/hash_set.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/entity/entity_set.rs b/crates/bevy_ecs/src/entity/entity_set.rs index 5202c6d9f82a5..e8246c75cedd5 100644 --- a/crates/bevy_ecs/src/entity/entity_set.rs +++ b/crates/bevy_ecs/src/entity/entity_set.rs @@ -3,7 +3,7 @@ use alloc::{ collections::{btree_map, btree_set}, rc::Rc, }; -use bevy_utils::hashbrown::HashSet; +use bevy_platform_support::collections::HashSet; use core::{ array, diff --git a/crates/bevy_ecs/src/entity/hash_set.rs b/crates/bevy_ecs/src/entity/hash_set.rs index dd368ff671014..84de962a82b76 100644 --- a/crates/bevy_ecs/src/entity/hash_set.rs +++ b/crates/bevy_ecs/src/entity/hash_set.rs @@ -12,9 +12,9 @@ use core::{ }, }; +use bevy_platform_support::collections::hash_set::{self, HashSet}; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; -use bevy_utils::hashbrown::hash_set::{self, HashSet}; use super::{Entity, EntityHash, EntitySet, EntitySetIterator, FromEntitySetIterator}; From 13eec29b94f951a2b5973baa326deb8b6ba33961 Mon Sep 17 00:00:00 2001 From: Victoron <59878206+Victoronz@users.noreply.github.com> Date: Thu, 23 Jan 2025 23:38:48 +0100 Subject: [PATCH 4/4] use fold instead of a for loop --- crates/bevy_ecs/src/entity/entity_set.rs | 12 +++++++----- crates/bevy_ecs/src/entity/hash_set.rs | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/entity/entity_set.rs b/crates/bevy_ecs/src/entity/entity_set.rs index e8246c75cedd5..6b69897806071 100644 --- a/crates/bevy_ecs/src/entity/entity_set.rs +++ b/crates/bevy_ecs/src/entity/entity_set.rs @@ -326,12 +326,14 @@ impl FromEntitySetItera { fn from_entity_set_iter>(set_iter: I) -> Self { let iter = set_iter.into_iter(); - let mut set = HashSet::::with_capacity_and_hasher(iter.size_hint().0, S::default()); - for e in iter { + let set = HashSet::::with_capacity_and_hasher(iter.size_hint().0, S::default()); + iter.fold(set, |mut set, e| { // SAFETY: Every element in self is unique. - unsafe { set.insert_unique_unchecked(e) }; - } - set + unsafe { + set.insert_unique_unchecked(e); + } + set + }) } } diff --git a/crates/bevy_ecs/src/entity/hash_set.rs b/crates/bevy_ecs/src/entity/hash_set.rs index ff721f0872805..41fdb33fa7ca1 100644 --- a/crates/bevy_ecs/src/entity/hash_set.rs +++ b/crates/bevy_ecs/src/entity/hash_set.rs @@ -198,12 +198,14 @@ impl FromIterator for EntityHashSet { impl FromEntitySetIterator for EntityHashSet { fn from_entity_set_iter>(set_iter: I) -> Self { let iter = set_iter.into_iter(); - let mut set = EntityHashSet::with_capacity(iter.size_hint().0); - for e in iter { + let set = EntityHashSet::with_capacity(iter.size_hint().0); + iter.fold(set, |mut set, e| { // SAFETY: Every element in self is unique. - unsafe { set.insert_unique_unchecked(e) }; - } - set + unsafe { + set.insert_unique_unchecked(e); + } + set + }) } }