Skip to content

Commit ecb7804

Browse files
committed
[ecs] Improve Commands performance (#2332)
# Objective - Currently `Commands` are quite slow due to the need to allocate for each command and wrap it in a `Box<dyn Command>`. - For example: ```rust fn my_system(mut cmds: Commands) { cmds.spawn().insert(42).insert(3.14); } ``` will have 3 separate `Box<dyn Command>` that need to be allocated and ran. ## Solution - Utilize a specialized data structure keyed `CommandQueueInner`. - The purpose of `CommandQueueInner` is to hold a collection of commands in contiguous memory. - This allows us to store each `Command` type contiguously in memory and quickly iterate through them and apply the `Command::write` trait function to each element.
1 parent de0d459 commit ecb7804

File tree

6 files changed

+257
-46
lines changed

6 files changed

+257
-46
lines changed

benches/benches/bevy_ecs/commands.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,14 @@ struct FakeCommandA;
8080
struct FakeCommandB(u64);
8181

8282
impl Command for FakeCommandA {
83-
fn write(self: Box<Self>, world: &mut World) {
83+
fn write(self, world: &mut World) {
8484
black_box(self);
8585
black_box(world);
8686
}
8787
}
8888

8989
impl Command for FakeCommandB {
90-
fn write(self: Box<Self>, world: &mut World) {
90+
fn write(self, world: &mut World) {
9191
black_box(self);
9292
black_box(world);
9393
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
use super::Command;
2+
use crate::world::World;
3+
4+
struct CommandMeta {
5+
offset: usize,
6+
func: unsafe fn(value: *mut u8, world: &mut World),
7+
}
8+
9+
/// A queue of [`Command`]s
10+
//
11+
// NOTE: [`CommandQueue`] is implemented via a `Vec<u8>` over a `Vec<Box<dyn Command>>`
12+
// as an optimization. Since commands are used frequently in systems as a way to spawn
13+
// entities/components/resources, and it's not currently possible to parallelize these
14+
// due to mutable [`World`] access, maximizing performance for [`CommandQueue`] is
15+
// preferred to simplicity of implementation.
16+
#[derive(Default)]
17+
pub struct CommandQueue {
18+
bytes: Vec<u8>,
19+
metas: Vec<CommandMeta>,
20+
}
21+
22+
// SAFE: All commands [`Command`] implement [`Send`]
23+
unsafe impl Send for CommandQueue {}
24+
25+
// SAFE: `&CommandQueue` never gives access to the inner commands.
26+
unsafe impl Sync for CommandQueue {}
27+
28+
impl CommandQueue {
29+
/// Push a [`Command`] onto the queue.
30+
#[inline]
31+
pub fn push<C>(&mut self, command: C)
32+
where
33+
C: Command,
34+
{
35+
/// SAFE: This function is only every called when the `command` bytes is the associated
36+
/// [`Commands`] `T` type. Also this only reads the data via `read_unaligned` so unaligned
37+
/// accesses are safe.
38+
unsafe fn write_command<T: Command>(command: *mut u8, world: &mut World) {
39+
let command = command.cast::<T>().read_unaligned();
40+
command.write(world);
41+
}
42+
43+
let size = std::mem::size_of::<C>();
44+
let old_len = self.bytes.len();
45+
46+
self.metas.push(CommandMeta {
47+
offset: old_len,
48+
func: write_command::<C>,
49+
});
50+
51+
if size > 0 {
52+
self.bytes.reserve(size);
53+
54+
// SAFE: The internal `bytes` vector has enough storage for the
55+
// command (see the call the `reserve` above), and the vector has
56+
// its length set appropriately.
57+
// Also `command` is forgotten at the end of this function so that
58+
// when `apply` is called later, a double `drop` does not occur.
59+
unsafe {
60+
std::ptr::copy_nonoverlapping(
61+
&command as *const C as *const u8,
62+
self.bytes.as_mut_ptr().add(old_len),
63+
size,
64+
);
65+
self.bytes.set_len(old_len + size);
66+
}
67+
}
68+
69+
std::mem::forget(command);
70+
}
71+
72+
/// Execute the queued [`Command`]s in the world.
73+
/// This clears the queue.
74+
#[inline]
75+
pub fn apply(&mut self, world: &mut World) {
76+
// flush the previously queued entities
77+
world.flush();
78+
79+
// SAFE: In the iteration below, `meta.func` will safely consume and drop each pushed command.
80+
// This operation is so that we can reuse the bytes `Vec<u8>`'s internal storage and prevent
81+
// unnecessary allocations.
82+
unsafe { self.bytes.set_len(0) };
83+
84+
let byte_ptr = if self.bytes.as_mut_ptr().is_null() {
85+
// SAFE: If the vector's buffer pointer is `null` this mean nothing has been pushed to its bytes.
86+
// This means either that:
87+
//
88+
// 1) There are no commands so this pointer will never be read/written from/to.
89+
//
90+
// 2) There are only zero-sized commands pushed.
91+
// According to https://doc.rust-lang.org/std/ptr/index.html
92+
// "The canonical way to obtain a pointer that is valid for zero-sized accesses is NonNull::dangling"
93+
// therefore it is safe to call `read_unaligned` on a pointer produced from `NonNull::dangling` for
94+
// zero-sized commands.
95+
unsafe { std::ptr::NonNull::dangling().as_mut() }
96+
} else {
97+
self.bytes.as_mut_ptr()
98+
};
99+
100+
for meta in self.metas.drain(..) {
101+
// SAFE: The implementation of `write_command` is safe for the according Command type.
102+
// The bytes are safely cast to their original type, safely read, and then dropped.
103+
unsafe {
104+
(meta.func)(byte_ptr.add(meta.offset), world);
105+
}
106+
}
107+
}
108+
}
109+
110+
#[cfg(test)]
111+
mod test {
112+
use super::*;
113+
use std::{
114+
panic::AssertUnwindSafe,
115+
sync::{
116+
atomic::{AtomicU32, Ordering},
117+
Arc,
118+
},
119+
};
120+
121+
struct DropCheck(Arc<AtomicU32>);
122+
123+
impl DropCheck {
124+
fn new() -> (Self, Arc<AtomicU32>) {
125+
let drops = Arc::new(AtomicU32::new(0));
126+
(Self(drops.clone()), drops)
127+
}
128+
}
129+
130+
impl Drop for DropCheck {
131+
fn drop(&mut self) {
132+
self.0.fetch_add(1, Ordering::Relaxed);
133+
}
134+
}
135+
136+
impl Command for DropCheck {
137+
fn write(self, _: &mut World) {}
138+
}
139+
140+
#[test]
141+
fn test_command_queue_inner_drop() {
142+
let mut queue = CommandQueue::default();
143+
144+
let (dropcheck_a, drops_a) = DropCheck::new();
145+
let (dropcheck_b, drops_b) = DropCheck::new();
146+
147+
queue.push(dropcheck_a);
148+
queue.push(dropcheck_b);
149+
150+
assert_eq!(drops_a.load(Ordering::Relaxed), 0);
151+
assert_eq!(drops_b.load(Ordering::Relaxed), 0);
152+
153+
let mut world = World::new();
154+
queue.apply(&mut world);
155+
156+
assert_eq!(drops_a.load(Ordering::Relaxed), 1);
157+
assert_eq!(drops_b.load(Ordering::Relaxed), 1);
158+
}
159+
160+
struct SpawnCommand;
161+
162+
impl Command for SpawnCommand {
163+
fn write(self, world: &mut World) {
164+
world.spawn();
165+
}
166+
}
167+
168+
#[test]
169+
fn test_command_queue_inner() {
170+
let mut queue = CommandQueue::default();
171+
172+
queue.push(SpawnCommand);
173+
queue.push(SpawnCommand);
174+
175+
let mut world = World::new();
176+
queue.apply(&mut world);
177+
178+
assert_eq!(world.entities().len(), 2);
179+
180+
// The previous call to `apply` cleared the queue.
181+
// This call should do nothing.
182+
queue.apply(&mut world);
183+
assert_eq!(world.entities().len(), 2);
184+
}
185+
186+
// This has an arbitrary value `String` stored to ensure
187+
// when then command gets pushed, the `bytes` vector gets
188+
// some data added to it.
189+
struct PanicCommand(String);
190+
impl Command for PanicCommand {
191+
fn write(self, _: &mut World) {
192+
panic!("command is panicking");
193+
}
194+
}
195+
196+
#[test]
197+
fn test_command_queue_inner_panic_safe() {
198+
std::panic::set_hook(Box::new(|_| {}));
199+
200+
let mut queue = CommandQueue::default();
201+
202+
queue.push(PanicCommand("I panic!".to_owned()));
203+
queue.push(SpawnCommand);
204+
205+
let mut world = World::new();
206+
207+
let _ = std::panic::catch_unwind(AssertUnwindSafe(|| {
208+
queue.apply(&mut world);
209+
}));
210+
211+
// even though the first command panicking.
212+
// the `bytes`/`metas` vectors were cleared.
213+
assert_eq!(queue.bytes.len(), 0);
214+
assert_eq!(queue.metas.len(), 0);
215+
216+
// Even though the first command panicked, it's still ok to push
217+
// more commands.
218+
queue.push(SpawnCommand);
219+
queue.push(SpawnCommand);
220+
queue.apply(&mut world);
221+
assert_eq!(world.entities().len(), 2);
222+
}
223+
224+
// NOTE: `CommandQueue` is `Send` because `Command` is send.
225+
// If the `Command` trait gets reworked to be non-send, `CommandQueue`
226+
// should be reworked.
227+
// This test asserts that Command types are send.
228+
fn assert_is_send_impl(_: impl Send) {}
229+
fn assert_is_send(command: impl Command) {
230+
assert_is_send_impl(command);
231+
}
232+
233+
#[test]
234+
fn test_command_is_send() {
235+
assert_is_send(SpawnCommand);
236+
}
237+
}

crates/bevy_ecs/src/system/commands.rs renamed to crates/bevy_ecs/src/system/commands/mod.rs

+13-39
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,18 @@
1+
mod command_queue;
2+
13
use crate::{
24
bundle::Bundle,
35
component::Component,
46
entity::{Entities, Entity},
57
world::World,
68
};
79
use bevy_utils::tracing::debug;
10+
pub use command_queue::CommandQueue;
811
use std::marker::PhantomData;
912

1013
/// A [`World`] mutation.
1114
pub trait Command: Send + Sync + 'static {
12-
fn write(self: Box<Self>, world: &mut World);
13-
}
14-
15-
/// A queue of [`Command`]s.
16-
#[derive(Default)]
17-
pub struct CommandQueue {
18-
commands: Vec<Box<dyn Command>>,
19-
}
20-
21-
impl CommandQueue {
22-
/// Execute the queued [`Command`]s in the world.
23-
/// This clears the queue.
24-
pub fn apply(&mut self, world: &mut World) {
25-
world.flush();
26-
for command in self.commands.drain(..) {
27-
command.write(world);
28-
}
29-
}
30-
31-
/// Push a boxed [`Command`] onto the queue.
32-
#[inline]
33-
pub fn push_boxed(&mut self, command: Box<dyn Command>) {
34-
self.commands.push(command);
35-
}
36-
37-
/// Push a [`Command`] onto the queue.
38-
#[inline]
39-
pub fn push<T: Command>(&mut self, command: T) {
40-
self.push_boxed(Box::new(command));
41-
}
15+
fn write(self, world: &mut World);
4216
}
4317

4418
/// A list of commands that will be run to modify a [`World`].
@@ -292,7 +266,7 @@ impl<T> Command for Spawn<T>
292266
where
293267
T: Bundle,
294268
{
295-
fn write(self: Box<Self>, world: &mut World) {
269+
fn write(self, world: &mut World) {
296270
world.spawn().insert_bundle(self.bundle);
297271
}
298272
}
@@ -310,7 +284,7 @@ where
310284
I: IntoIterator + Send + Sync + 'static,
311285
I::Item: Bundle,
312286
{
313-
fn write(self: Box<Self>, world: &mut World) {
287+
fn write(self, world: &mut World) {
314288
world.spawn_batch(self.bundles_iter);
315289
}
316290
}
@@ -321,7 +295,7 @@ pub struct Despawn {
321295
}
322296

323297
impl Command for Despawn {
324-
fn write(self: Box<Self>, world: &mut World) {
298+
fn write(self, world: &mut World) {
325299
if !world.despawn(self.entity) {
326300
debug!("Failed to despawn non-existent entity {:?}", self.entity);
327301
}
@@ -337,7 +311,7 @@ impl<T> Command for InsertBundle<T>
337311
where
338312
T: Bundle + 'static,
339313
{
340-
fn write(self: Box<Self>, world: &mut World) {
314+
fn write(self, world: &mut World) {
341315
world.entity_mut(self.entity).insert_bundle(self.bundle);
342316
}
343317
}
@@ -352,7 +326,7 @@ impl<T> Command for Insert<T>
352326
where
353327
T: Component,
354328
{
355-
fn write(self: Box<Self>, world: &mut World) {
329+
fn write(self, world: &mut World) {
356330
world.entity_mut(self.entity).insert(self.component);
357331
}
358332
}
@@ -367,7 +341,7 @@ impl<T> Command for Remove<T>
367341
where
368342
T: Component,
369343
{
370-
fn write(self: Box<Self>, world: &mut World) {
344+
fn write(self, world: &mut World) {
371345
if let Some(mut entity_mut) = world.get_entity_mut(self.entity) {
372346
entity_mut.remove::<T>();
373347
}
@@ -384,7 +358,7 @@ impl<T> Command for RemoveBundle<T>
384358
where
385359
T: Bundle,
386360
{
387-
fn write(self: Box<Self>, world: &mut World) {
361+
fn write(self, world: &mut World) {
388362
if let Some(mut entity_mut) = world.get_entity_mut(self.entity) {
389363
// remove intersection to gracefully handle components that were removed before running
390364
// this command
@@ -398,7 +372,7 @@ pub struct InsertResource<T: Component> {
398372
}
399373

400374
impl<T: Component> Command for InsertResource<T> {
401-
fn write(self: Box<Self>, world: &mut World) {
375+
fn write(self, world: &mut World) {
402376
world.insert_resource(self.resource);
403377
}
404378
}
@@ -408,7 +382,7 @@ pub struct RemoveResource<T: Component> {
408382
}
409383

410384
impl<T: Component> Command for RemoveResource<T> {
411-
fn write(self: Box<Self>, world: &mut World) {
385+
fn write(self, world: &mut World) {
412386
world.remove_resource::<T>();
413387
}
414388
}

0 commit comments

Comments
 (0)