Skip to content

Commit dc70b2e

Browse files
cartcart
authored
Insert ResolvedScenes as dynamic bundles (#23808)
# Objective Currently BSN inserts each component one-by-one. This is incredibly expensive, as it forces an archetype move after every insert. ## Solution Scene templates now write their outputs to reusable bundle scratch space (which uses a bump allocator). The final bundle is written after all components (including inherited components) are written to the bundle. - Introduce a `BundleWriter`, which enables defining a dynamic bundle using scratch space in a bump allocator. Currently this only supports writing individual components to the `BundleWriter`, because supporting arbitrary bundles is much harder (ex: dynamic bundle effects). This sadly means that we are temporarily constraining both `bsn! { Node }` and `bsn! { @SomeTemplate }` "template patches" to _require_ a component output. Custom scene impls can still push arbitrary bundles, which are inserted before the final dynamic bundle write. In practice I believe this will cover the relevant use cases in the short term. - We now skip duplicate insertions of components when spawning inherited scenes. - We now write empty RelationshipTarget collections, pre-allocated to the correct size to the dynamic bundle, which both avoids another archetype move and ensures we only allocate the inner relationship target collection once. - We now write the Relationship component to the dynamic bundle, avoiding an archetype move - I added a new "loaded scene inheritance" test variant, just to make sure that case still works - `ErasedTemplate` has been moved to `bevy_scene`, as it is now "opinionated", more specific to `bevy_scene`, and less safe to use in a general context <img width="795" height="274" alt="image" src="https://github.com/user-attachments/assets/68ea02c7-6b7e-4f7c-9474-603188fe6d0b" /> These are benchmarks that produce the same UI scene hierarchy through different means (the benchmarks have been updated to have a few "matrix wrapper" components to show the cost of archetype moves): - `immediate_function_scene`: a test of going through the whole "scene building" process, then spawning. this is the cost of ad-hoc scenes that don't reuse work from inherited scenes - `immediate_loaded_scene`: a test where we instantiate a bunch of inherited scene instances, where the inherited scene has already been computed / cached. - `raw_bundle_no_scene`: just spawning the raw bundle directly
1 parent 85212f7 commit dc70b2e

11 files changed

Lines changed: 881 additions & 154 deletions

File tree

‎benches/benches/bevy_scene/spawn.rs‎

Lines changed: 91 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use bevy_reflect::TypePath;
22
use criterion::{criterion_group, Criterion};
3+
use glam::Mat4;
34
use std::{path::Path, time::Duration};
45

56
use bevy_app::App;
@@ -56,6 +57,16 @@ fn ui_loaded_asset() -> impl Scene {
5657
#[derive(Component, Default, Clone)]
5758
struct Marker;
5859

60+
#[derive(Component, Default, Clone)]
61+
#[expect(unused, reason = "this exists to take up space")]
62+
struct Marker1(Mat4);
63+
#[derive(Component, Default, Clone)]
64+
#[expect(unused, reason = "this exists to take up space")]
65+
struct Marker2(Mat4);
66+
#[derive(Component, Default, Clone)]
67+
#[expect(unused, reason = "this exists to take up space")]
68+
struct Marker3(Mat4);
69+
5970
fn button() -> impl Scene {
6071
bsn! {
6172
Button
@@ -67,16 +78,16 @@ fn button() -> impl Scene {
6778
align_items: AlignItems::Center,
6879
}
6980
Children [
70-
(Text("Text") Marker),
71-
(Text("Text") Marker),
72-
(Text("Text") Marker),
73-
(Text("Text") Marker),
74-
(Text("Text") Marker),
75-
(Text("Text") Marker),
76-
(Text("Text") Marker),
77-
(Text("Text") Marker),
78-
(Text("Text") Marker),
79-
(Text("Text") Marker),
81+
(Text("Text") Marker Marker1 Marker2 Marker3),
82+
(Text("Text") Marker Marker1 Marker2 Marker3),
83+
(Text("Text") Marker Marker1 Marker2 Marker3),
84+
(Text("Text") Marker Marker1 Marker2 Marker3),
85+
(Text("Text") Marker Marker1 Marker2 Marker3),
86+
(Text("Text") Marker Marker1 Marker2 Marker3),
87+
(Text("Text") Marker Marker1 Marker2 Marker3),
88+
(Text("Text") Marker Marker1 Marker2 Marker3),
89+
(Text("Text") Marker Marker1 Marker2 Marker3),
90+
(Text("Text") Marker Marker1 Marker2 Marker3),
8091
]
8192
}
8293
}
@@ -93,16 +104,76 @@ fn raw_button() -> impl Bundle {
93104
..Default::default()
94105
},
95106
children![
96-
(Text("Text".into()), Marker),
97-
(Text("Text".into()), Marker),
98-
(Text("Text".into()), Marker),
99-
(Text("Text".into()), Marker),
100-
(Text("Text".into()), Marker),
101-
(Text("Text".into()), Marker),
102-
(Text("Text".into()), Marker),
103-
(Text("Text".into()), Marker),
104-
(Text("Text".into()), Marker),
105-
(Text("Text".into()), Marker),
107+
(
108+
Text("Text".into()),
109+
Marker,
110+
Marker1::default(),
111+
Marker2::default(),
112+
Marker3::default()
113+
),
114+
(
115+
Text("Text".into()),
116+
Marker,
117+
Marker1::default(),
118+
Marker2::default(),
119+
Marker3::default()
120+
),
121+
(
122+
Text("Text".into()),
123+
Marker,
124+
Marker1::default(),
125+
Marker2::default(),
126+
Marker3::default()
127+
),
128+
(
129+
Text("Text".into()),
130+
Marker,
131+
Marker1::default(),
132+
Marker2::default(),
133+
Marker3::default()
134+
),
135+
(
136+
Text("Text".into()),
137+
Marker,
138+
Marker1::default(),
139+
Marker2::default(),
140+
Marker3::default()
141+
),
142+
(
143+
Text("Text".into()),
144+
Marker,
145+
Marker1::default(),
146+
Marker2::default(),
147+
Marker3::default()
148+
),
149+
(
150+
Text("Text".into()),
151+
Marker,
152+
Marker1::default(),
153+
Marker2::default(),
154+
Marker3::default()
155+
),
156+
(
157+
Text("Text".into()),
158+
Marker,
159+
Marker1::default(),
160+
Marker2::default(),
161+
Marker3::default()
162+
),
163+
(
164+
Text("Text".into()),
165+
Marker,
166+
Marker1::default(),
167+
Marker2::default(),
168+
Marker3::default()
169+
),
170+
(
171+
Text("Text".into()),
172+
Marker,
173+
Marker1::default(),
174+
Marker2::default(),
175+
Marker3::default()
176+
),
106177
],
107178
)
108179
}

‎crates/bevy_ecs/Cargo.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ std = [
7676
"arrayvec/std",
7777
"log/std",
7878
"bevy_platform/std",
79-
"downcast-rs/std",
8079
]
8180

8281
## `critical-section` provides the building blocks for synchronization primitives
@@ -127,7 +126,6 @@ log = { version = "0.4", default-features = false }
127126
bumpalo = "3"
128127
subsecond = { version = "0.7.0-rc.0", optional = true }
129128
slotmap = { version = "1.0.7", default-features = false }
130-
downcast-rs = { version = "2", default-features = false }
131129

132130
concurrent-queue = { version = "2.5.0", default-features = false }
133131
[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies]

‎crates/bevy_ecs/src/bundle/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod remove;
1414
mod spawner;
1515
#[cfg(test)]
1616
mod tests;
17+
mod writer;
1718

1819
pub(crate) use insert::BundleInserter;
1920
pub(crate) use remove::BundleRemover;
@@ -22,6 +23,7 @@ pub(crate) use spawner::BundleSpawner;
2223
use bevy_ptr::MovingPtr;
2324
use core::mem::MaybeUninit;
2425
pub use info::*;
26+
pub use writer::*;
2527

2628
/// Derive the [`Bundle`] trait
2729
///

‎crates/bevy_ecs/src/bundle/writer.rs‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
use crate::{
2+
component::{Component, ComponentId, Components, ComponentsRegistrator},
3+
relationship::RelationshipHookMode,
4+
world::EntityWorldMut,
5+
};
6+
use alloc::vec::Vec;
7+
use bevy_ptr::OwningPtr;
8+
use bumpalo::Bump;
9+
use core::{alloc::Layout, ptr::NonNull};
10+
11+
/// Enables pushing components to internal scratch space (uses a bump allocator), which can then be
12+
/// written as a dynamic bundle. The contents are cleared after each write and the allocated scratch
13+
/// space is reused across writes.
14+
///
15+
/// Also see [`BundleWriter`].
16+
#[derive(Default)]
17+
pub struct BundleScratch {
18+
// Correctness: this should never be made public or mismatched component ids could be inserted
19+
component_ids: Vec<ComponentId>,
20+
// Correctness: this should never be made public or arbitrary non-components could be inserted
21+
component_ptrs: Vec<NonNull<u8>>,
22+
// Safety: this cannot be exposed, otherwise `alloc.reset()` could be called in arbitrary places,
23+
// which could invalidate the data stored in `component_ptrs`.
24+
alloc: Bump,
25+
}
26+
27+
impl BundleScratch {
28+
/// Creates a new [`BundleWriter`] using this scratch space. For safety / correctness, this will
29+
/// clear any existing components.
30+
///
31+
/// Note that for performance reasons this will _not_ clear the internal allocator. To avoid leaking,
32+
/// make sure every component pushed to the [`BundleWriter`] is followed by either a
33+
/// [`BundleWriter::write`] or a [`BundleScratch::manual_drop`].
34+
#[inline]
35+
pub fn writer<'a>(&'a mut self) -> BundleWriter<'a> {
36+
// This is necessary to ensure safety / correctness is maintained in the context of catch_unwind
37+
// or a skipped `write`
38+
self.component_ids.clear();
39+
self.component_ptrs.clear();
40+
BundleWriter(self)
41+
}
42+
43+
/// Returns true if there are currently no components stored in the scratch space.
44+
#[inline]
45+
pub fn is_empty(&self) -> bool {
46+
self.component_ids.is_empty()
47+
}
48+
49+
/// This will drop all components currently stored in the scratch space. This is generally used to
50+
/// ensure drops occur in error scenarios.
51+
///
52+
/// # Safety
53+
/// `components` must be from the same world as the components that were pushed to this writer.
54+
pub unsafe fn manual_drop(&mut self, components: &Components) {
55+
for (id, ptr) in self
56+
.component_ids
57+
.drain(..)
58+
.zip(self.component_ptrs.drain(..))
59+
{
60+
if let Some(info) = components.get_info(id)
61+
&& let Some(drop) = info.drop()
62+
{
63+
// SAFETY: ptr is a valid component that matches the given component id
64+
unsafe {
65+
let ptr = OwningPtr::new(ptr);
66+
(drop)(ptr);
67+
}
68+
}
69+
}
70+
self.alloc.reset();
71+
}
72+
}
73+
74+
/// Enables pushing components to the internal [`BundleScratch`], which can then be
75+
/// written as a dynamic bundle.
76+
///
77+
/// Components pushed to this writer should either be followed by a [`BundleWriter::write`] or a
78+
/// [`BundleScratch::manual_drop`] to avoid leaking.
79+
pub struct BundleWriter<'a>(&'a mut BundleScratch);
80+
81+
// SAFETY: The `NonNull`s in component_ptrs are always a `Component`, which is Send
82+
unsafe impl Send for BundleScratch where Bump: Send {}
83+
84+
impl<'a> BundleWriter<'a> {
85+
/// Pushes the given component to the back of the current bundle scratch space. It will register
86+
/// the component in `components` if it does not already exist.
87+
///
88+
/// # Safety
89+
///
90+
/// `components` must be from the same world that all previous [`Self::push_component`] calls were called with,
91+
/// and the _next_ [`Self::write`] call.
92+
pub unsafe fn push_component<C: Component>(
93+
&mut self,
94+
components: &mut ComponentsRegistrator,
95+
component: C,
96+
) {
97+
let id = components.register_component::<C>();
98+
OwningPtr::make(component, |ptr| {
99+
// SAFETY: ptr points to a C component value which matches the `id` looked up above.
100+
// Layout matches C.
101+
self.push_component_by_id(id, ptr, Layout::new::<C>());
102+
});
103+
}
104+
105+
/// Pushes the given component ptr to the back of the current bundle scratch space.
106+
///
107+
/// # Safety
108+
///
109+
/// `components` must be from the same world that all previous [`Self::push_component`] calls were called with,
110+
/// and the _next_ [`Self::write`] call. `component` must point to a [`Component`] value that matches `id`.
111+
/// `layout` must correspond to the layout of the [`Component`] type.
112+
pub unsafe fn push_component_by_id(
113+
&mut self,
114+
id: ComponentId,
115+
component: OwningPtr<'_>,
116+
layout: Layout,
117+
) {
118+
let ptr = self.0.alloc.alloc_layout(layout);
119+
core::ptr::copy(component.as_ptr(), ptr.as_ptr(), layout.size());
120+
self.0.component_ids.push(id);
121+
self.0.component_ptrs.push(ptr);
122+
}
123+
124+
/// Writes the current contents of the bundle to the given `entity` and clears the scratch space.
125+
///
126+
/// # Safety
127+
///
128+
/// `entity` must be from the same world that all [`Self::push_component`] calls since the last
129+
/// [`Self::write`] were called with.
130+
pub unsafe fn write(self, entity: &mut EntityWorldMut) {
131+
// SAFETY:
132+
// - All `component_ids` are from the same world as `entity`
133+
// - All `component_data_ptrs` are valid types represented by `component_ids`
134+
unsafe {
135+
entity.insert_by_ids_internal(
136+
&self.0.component_ids,
137+
self.0
138+
.component_ptrs
139+
.drain(..)
140+
.map(|ptr| OwningPtr::new(ptr)),
141+
RelationshipHookMode::Run,
142+
);
143+
}
144+
self.0.component_ids.clear();
145+
self.0.alloc.reset();
146+
}
147+
148+
/// Returns true if there are currently no components.
149+
#[inline]
150+
pub fn is_empty(&self) -> bool {
151+
self.0.component_ids.is_empty()
152+
}
153+
}
154+
155+
#[cfg(test)]
156+
mod tests {
157+
use crate::{bundle::BundleScratch, component::Component, name::Name, world::World};
158+
159+
#[test]
160+
fn write_component() {
161+
#[derive(Component)]
162+
struct X;
163+
164+
let mut world = World::new();
165+
let mut bundle_scratch = BundleScratch::default();
166+
let mut bundle_writer = bundle_scratch.writer();
167+
// SAFETY: the same world is used for every bundle_writer operation
168+
unsafe {
169+
let mut components = world.components_registrator();
170+
bundle_writer.push_component(&mut components, X);
171+
bundle_writer.push_component(&mut components, Name::new("Hi"));
172+
let mut entity = world.spawn_empty();
173+
bundle_writer.write(&mut entity);
174+
175+
assert_eq!(entity.get::<Name>().unwrap().as_str(), "Hi");
176+
assert!(entity.contains::<X>());
177+
}
178+
}
179+
}

0 commit comments

Comments
 (0)