Skip to content

Commit 8b36cca

Browse files
authored
Combine added and existing ComponentIds in a single allocation (#21102)
# Objective Further shrink `ArchetypeAfterBundleInsert`, following #20626. Remove the need for a `collect()` when triggering `Insert` observers. ## Solution Combine the `added` and `existing` component id lists into a single boxed slice. We need to store an additional length to recover the slices, but this means we store **one** pointer and two lengths instead of **two** pointers and two lengths, shrinking the size of `ArchetypeAfterBundleInsert` by one pointer. This also means we have a slice available for the full list of `inserted` components without having to copy them into a new `Vec`.
1 parent 870c8ca commit 8b36cca

File tree

2 files changed

+31
-27
lines changed

2 files changed

+31
-27
lines changed

crates/bevy_ecs/src/archetype.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use crate::{
2525
entity::{Entity, EntityLocation},
2626
event::Event,
2727
observer::Observers,
28+
query::DebugCheckedUnwrap,
2829
storage::{ImmutableSparseSet, SparseArray, SparseSet, TableId, TableRow},
2930
};
3031
use alloc::{boxed::Box, vec::Vec};
@@ -142,24 +143,27 @@ pub(crate) struct ArchetypeAfterBundleInsert {
142143
///
143144
/// The initial values are determined based on the provided constructor, falling back to the `Default` trait if none is given.
144145
pub required_components: Box<[RequiredComponentConstructor]>,
145-
/// The components added by this bundle. This includes any Required Components that are inserted when adding this bundle.
146-
pub(crate) added: Box<[ComponentId]>,
147-
/// The components that were explicitly contributed by this bundle, but already existed in the archetype. This _does not_ include any
148-
/// Required Components.
149-
pub(crate) existing: Box<[ComponentId]>,
146+
/// The components inserted by this bundle, with added components before existing ones.
147+
/// Added components includes any Required Components that are inserted when adding this bundle,
148+
/// but existing components only includes ones explicitly contributed by this bundle.
149+
inserted: Box<[ComponentId]>,
150+
/// The number of components added by this bundle, including Required Components.
151+
added_len: usize,
150152
}
151153

152154
impl ArchetypeAfterBundleInsert {
153-
pub(crate) fn iter_inserted(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
154-
self.added.iter().chain(self.existing.iter()).copied()
155+
pub(crate) fn inserted(&self) -> &[ComponentId] {
156+
&self.inserted
155157
}
156158

157-
pub(crate) fn iter_added(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
158-
self.added.iter().copied()
159+
pub(crate) fn added(&self) -> &[ComponentId] {
160+
// SAFETY: `added_len` is always in range `0..=inserted.len()`
161+
unsafe { self.inserted.get(..self.added_len).debug_checked_unwrap() }
159162
}
160163

161-
pub(crate) fn iter_existing(&self) -> impl Iterator<Item = ComponentId> + Clone + '_ {
162-
self.existing.iter().copied()
164+
pub(crate) fn existing(&self) -> &[ComponentId] {
165+
// SAFETY: `added_len` is always in range `0..=inserted.len()`
166+
unsafe { self.inserted.get(self.added_len..).debug_checked_unwrap() }
163167
}
164168
}
165169

@@ -244,17 +248,21 @@ impl Edges {
244248
archetype_id: ArchetypeId,
245249
bundle_status: impl Into<Box<[ComponentStatus]>>,
246250
required_components: impl Into<Box<[RequiredComponentConstructor]>>,
247-
added: impl Into<Box<[ComponentId]>>,
248-
existing: impl Into<Box<[ComponentId]>>,
251+
mut added: Vec<ComponentId>,
252+
existing: Vec<ComponentId>,
249253
) {
254+
let added_len = added.len();
255+
// Make sure `extend` doesn't over-reserve, since the conversion to `Box<[_]>` would reallocate to shrink.
256+
added.reserve_exact(existing.len());
257+
added.extend(existing);
250258
self.insert_bundle.insert(
251259
bundle_id,
252260
ArchetypeAfterBundleInsert {
253261
archetype_id,
254262
bundle_status: bundle_status.into(),
255263
required_components: required_components.into(),
256-
added: added.into(),
257-
existing: existing.into(),
264+
added_len,
265+
inserted: added.into(),
258266
},
259267
);
260268
}

crates/bevy_ecs/src/bundle/insert.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,15 @@ impl<'w> BundleInserter<'w> {
174174
REPLACE,
175175
&mut Replace { entity },
176176
&mut EntityComponentsTrigger {
177-
components: &archetype_after_insert.existing,
177+
components: archetype_after_insert.existing(),
178178
},
179179
caller,
180180
);
181181
}
182182
deferred_world.trigger_on_replace(
183183
archetype,
184184
entity,
185-
archetype_after_insert.iter_existing(),
185+
archetype_after_insert.existing().iter().copied(),
186186
caller,
187187
relationship_hook_mode,
188188
);
@@ -353,7 +353,7 @@ impl<'w> BundleInserter<'w> {
353353
deferred_world.trigger_on_add(
354354
new_archetype,
355355
entity,
356-
archetype_after_insert.iter_added(),
356+
archetype_after_insert.added().iter().copied(),
357357
caller,
358358
);
359359
if new_archetype.has_add_observer() {
@@ -362,7 +362,7 @@ impl<'w> BundleInserter<'w> {
362362
ADD,
363363
&mut Add { entity },
364364
&mut EntityComponentsTrigger {
365-
components: &archetype_after_insert.added,
365+
components: archetype_after_insert.added(),
366366
},
367367
caller,
368368
);
@@ -373,7 +373,7 @@ impl<'w> BundleInserter<'w> {
373373
deferred_world.trigger_on_insert(
374374
new_archetype,
375375
entity,
376-
archetype_after_insert.iter_inserted(),
376+
archetype_after_insert.inserted().iter().copied(),
377377
caller,
378378
relationship_hook_mode,
379379
);
@@ -382,12 +382,8 @@ impl<'w> BundleInserter<'w> {
382382
deferred_world.trigger_raw(
383383
INSERT,
384384
&mut Insert { entity },
385-
// PERF: this is not a regression from what we were doing before, but ideally we don't
386-
// need to collect here
387385
&mut EntityComponentsTrigger {
388-
components: &archetype_after_insert
389-
.iter_inserted()
390-
.collect::<Vec<_>>(),
386+
components: archetype_after_insert.inserted(),
391387
},
392388
caller,
393389
);
@@ -399,7 +395,7 @@ impl<'w> BundleInserter<'w> {
399395
deferred_world.trigger_on_insert(
400396
new_archetype,
401397
entity,
402-
archetype_after_insert.iter_added(),
398+
archetype_after_insert.added().iter().copied(),
403399
caller,
404400
relationship_hook_mode,
405401
);
@@ -409,7 +405,7 @@ impl<'w> BundleInserter<'w> {
409405
INSERT,
410406
&mut Insert { entity },
411407
&mut EntityComponentsTrigger {
412-
components: &archetype_after_insert.added,
408+
components: archetype_after_insert.added(),
413409
},
414410
caller,
415411
);

0 commit comments

Comments
 (0)