From 7d5ac3bc61d68700dffbfbca810e5f05a8ab4d1e Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Wed, 18 Dec 2024 20:00:36 -0800 Subject: [PATCH 01/13] add without's so systems can run in parallel when possible --- examples/stress_tests/many_components.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 4bb87d322d5c7..32fbf6a5e4c05 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -107,6 +107,11 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .choose_multiple(&mut rng, num_access_components) .copied() .collect(); + let without_components: Vec = component_ids + .iter() + .filter(|component_id| !access_components.contains(component_id)) + .copied() + .collect(); let system = (QueryParamBuilder::new(|builder| { for &access_component in &access_components { if rand::random::() { @@ -115,6 +120,10 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { builder.ref_id(access_component); } } + + for &without_component in &without_components { + builder.without_id(without_component); + } }),) .build_state(world) .build_any_system(base_system); From d8def7bc4e4686867b578e55a771d53cd70f9496 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Wed, 18 Dec 2024 22:15:04 -0800 Subject: [PATCH 02/13] add a span --- examples/stress_tests/many_components.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 32fbf6a5e4c05..503c8048a5920 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -21,7 +21,7 @@ use bevy::{ system::QueryParamBuilder, world::FilteredEntityMut, }, - log::LogPlugin, + log::{info_span, LogPlugin}, prelude::{App, In, IntoSystem, Query, Schedule, SystemParamBuilder, Update}, ptr::OwningPtr, MinimalPlugins, @@ -34,6 +34,7 @@ use std::{alloc::Layout, num::Wrapping}; // A simple system that matches against several components and does some menial calculation to create // some non-trivial load. fn base_system(access_components: In>, mut query: Query) { + let _span = info_span!("base_system", components = ?access_components.0).entered(); for mut filtered_entity in &mut query { // We calculate Faulhaber's formula mod 256 with n = value and p = exponent. // See https://en.wikipedia.org/wiki/Faulhaber%27s_formula From b0d5eff2d537bf72080249af4c409c4dfce9b2e7 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 19 Dec 2024 10:51:15 -0800 Subject: [PATCH 03/13] switch to use insert_by_ids --- examples/stress_tests/many_components.rs | 50 +++++++++++++++--------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 503c8048a5920..bb07c3677cae3 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -29,12 +29,12 @@ use bevy::{ use rand::prelude::{Rng, SeedableRng, SliceRandom}; use rand_chacha::ChaCha8Rng; -use std::{alloc::Layout, num::Wrapping}; +use std::{alloc::Layout, num::Wrapping, ptr::NonNull}; // A simple system that matches against several components and does some menial calculation to create // some non-trivial load. fn base_system(access_components: In>, mut query: Query) { - let _span = info_span!("base_system", components = ?access_components.0).entered(); + let _span = info_span!("base_system", components = ?access_components.0, count = query.iter().len()).entered(); for mut filtered_entity in &mut query { // We calculate Faulhaber's formula mod 256 with n = value and p = exponent. // See https://en.wikipedia.org/wiki/Faulhaber%27s_formula @@ -108,11 +108,11 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .choose_multiple(&mut rng, num_access_components) .copied() .collect(); - let without_components: Vec = component_ids - .iter() - .filter(|component_id| !access_components.contains(component_id)) - .copied() - .collect(); + // let without_components: Vec = component_ids + // .iter() + // .filter(|component_id| !access_components.contains(component_id)) + // .copied() + // .collect(); let system = (QueryParamBuilder::new(|builder| { for &access_component in &access_components { if rand::random::() { @@ -122,9 +122,9 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { } } - for &without_component in &without_components { - builder.without_id(without_component); - } + // for &without_component in &without_components { + // builder.without_id(without_component); + // } }),) .build_state(world) .build_any_system(base_system); @@ -134,20 +134,32 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { // spawn a bunch of entities for _ in 1..=num_entities { let num_components = rng.gen_range(1..10); - let components = component_ids.choose_multiple(&mut rng, num_components); + let components: Vec = component_ids + .choose_multiple(&mut rng, num_components) + .copied() + .collect(); let mut entity = world.spawn_empty(); - for &component_id in components { - let value: u8 = rng.gen_range(0..255); - OwningPtr::make(value, |ptr| { + let values: Vec = components + .iter() + .map(|_id| { + let mut value: u8 = rng.gen_range(0..255); + let value = &mut value; + // SAFETY: value is initialized above + #[allow(unsafe_code)] + let ptr = unsafe { NonNull::new_unchecked(value as *mut u8) }; #[allow(unsafe_code)] - // SAFETY: - // component_id is from the same world - // value is u8, so ptr is a valid reference for component_id unsafe { - entity.insert_by_id(component_id, ptr); + OwningPtr::new(ptr) } - }); + }) + .collect(); + // SAFETY: + // * component_id is from the same world + // * value is u8, so ptr is a valid reference for component_id + #[allow(unsafe_code)] + unsafe { + entity.insert_by_ids(&components, values.into_iter()); } } From bdf957ec5156e37ad2a82b61befec01108bfb242 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 19 Dec 2024 14:58:51 -0800 Subject: [PATCH 04/13] remove without's --- examples/stress_tests/many_components.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index bb07c3677cae3..83fcfa4b0b4c1 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -108,11 +108,6 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .choose_multiple(&mut rng, num_access_components) .copied() .collect(); - // let without_components: Vec = component_ids - // .iter() - // .filter(|component_id| !access_components.contains(component_id)) - // .copied() - // .collect(); let system = (QueryParamBuilder::new(|builder| { for &access_component in &access_components { if rand::random::() { @@ -121,10 +116,6 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { builder.ref_id(access_component); } } - - // for &without_component in &without_components { - // builder.without_id(without_component); - // } }),) .build_state(world) .build_any_system(base_system); From e133232399a4e2e3302ae797d9d695e72d77789e Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 19 Dec 2024 15:23:25 -0800 Subject: [PATCH 05/13] put span behind feature flag --- examples/stress_tests/many_components.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 83fcfa4b0b4c1..fe415c493ca6b 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -21,7 +21,7 @@ use bevy::{ system::QueryParamBuilder, world::FilteredEntityMut, }, - log::{info_span, LogPlugin}, + log::LogPlugin, prelude::{App, In, IntoSystem, Query, Schedule, SystemParamBuilder, Update}, ptr::OwningPtr, MinimalPlugins, @@ -34,7 +34,9 @@ use std::{alloc::Layout, num::Wrapping, ptr::NonNull}; // A simple system that matches against several components and does some menial calculation to create // some non-trivial load. fn base_system(access_components: In>, mut query: Query) { - let _span = info_span!("base_system", components = ?access_components.0, count = query.iter().len()).entered(); + #[cfg(feature = "trace")] + let _span = bevy::utils::tracing::info_span!("base_system", components = ?access_components.0, count = query.iter().len()).entered(); + for mut filtered_entity in &mut query { // We calculate Faulhaber's formula mod 256 with n = value and p = exponent. // See https://en.wikipedia.org/wiki/Faulhaber%27s_formula From 32cde8a5d46346b5d148dc1387a249c2c9446a25 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Thu, 19 Dec 2024 15:49:32 -0800 Subject: [PATCH 06/13] add number of archetype components --- examples/stress_tests/many_components.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index fe415c493ca6b..2c32a4496f744 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -156,6 +156,8 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { } } + println!("Number of Archetype-Components: {}", world.archetypes().archetype_components_len()); + // overwrite Update schedule in the app app.add_schedule(schedule); app.add_plugins(MinimalPlugins) From fdf67a01005eb1b3885481b3a0ca10b2a40b9b3c Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 20 Dec 2024 09:52:55 -0800 Subject: [PATCH 07/13] clean up safety comments --- examples/stress_tests/many_components.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 2c32a4496f744..8bc90f04a9fc3 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -87,8 +87,8 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { world.register_component_with_descriptor( #[allow(unsafe_code)] // SAFETY: - // we don't implement a drop function - // u8 is Sync and Send + // * We don't implement a drop function + // * u8 is Sync and Send unsafe { ComponentDescriptor::new_with_layout( format!("Component{}", i).to_string(), @@ -138,7 +138,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .map(|_id| { let mut value: u8 = rng.gen_range(0..255); let value = &mut value; - // SAFETY: value is initialized above + // SAFETY: value is non null since it was initialized from an &mut above #[allow(unsafe_code)] let ptr = unsafe { NonNull::new_unchecked(value as *mut u8) }; #[allow(unsafe_code)] @@ -149,7 +149,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .collect(); // SAFETY: // * component_id is from the same world - // * value is u8, so ptr is a valid reference for component_id + // * values was initialized above, so references are valid #[allow(unsafe_code)] unsafe { entity.insert_by_ids(&components, values.into_iter()); From 9af6cddd4c6ce1760688d2dd45badad76102e3c8 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 20 Dec 2024 10:47:47 -0800 Subject: [PATCH 08/13] fmt --- examples/stress_tests/many_components.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 8bc90f04a9fc3..cdb5b6a064a60 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -156,7 +156,10 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { } } - println!("Number of Archetype-Components: {}", world.archetypes().archetype_components_len()); + println!( + "Number of Archetype-Components: {}", + world.archetypes().archetype_components_len() + ); // overwrite Update schedule in the app app.add_schedule(schedule); From 62ca03adedb080c4611c2c353503a17e819c784d Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 20 Dec 2024 11:03:44 -0800 Subject: [PATCH 09/13] add a safety commment --- examples/stress_tests/many_components.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index cdb5b6a064a60..1014609f543a4 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -137,10 +137,13 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .iter() .map(|_id| { let mut value: u8 = rng.gen_range(0..255); - let value = &mut value; + let value = &mut value as *mut u8; // SAFETY: value is non null since it was initialized from an &mut above #[allow(unsafe_code)] - let ptr = unsafe { NonNull::new_unchecked(value as *mut u8) }; + let ptr = unsafe { NonNull::new_unchecked(value) }; + // SAFETY: + // * ptr was created from a u8, so is valid, aligned, and has correct provenance. + // * ptr was created from an exclusive reference, so nothing else can read or write the value #[allow(unsafe_code)] unsafe { OwningPtr::new(ptr) From 27ef2e5c80dea860fdc92f06edf6d09cca856e18 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 20 Dec 2024 12:17:23 -0800 Subject: [PATCH 10/13] fix UB by storing as a Vec> --- examples/stress_tests/many_components.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 1014609f543a4..33be7ef04ea2c 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -29,7 +29,7 @@ use bevy::{ use rand::prelude::{Rng, SeedableRng, SliceRandom}; use rand_chacha::ChaCha8Rng; -use std::{alloc::Layout, num::Wrapping, ptr::NonNull}; +use std::{alloc::Layout, mem::ManuallyDrop, num::Wrapping, ptr::NonNull}; // A simple system that matches against several components and does some menial calculation to create // some non-trivial load. @@ -133,14 +133,20 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .collect(); let mut entity = world.spawn_empty(); - let values: Vec = components + // We need to avoid dropping the u8's when `values` is dropped since ownership of the values + // is passed to the world. But we do want to deallocate the memory. + let mut values: Vec> = components .iter() - .map(|_id| { - let mut value: u8 = rng.gen_range(0..255); - let value = &mut value as *mut u8; + .map(|_id| ManuallyDrop::new(rng.gen_range(0..255))) + .collect(); + let ptrs: Vec = values + .iter_mut() + .map(|value| { + // ManuallyDrop is repr(transparent) so casting it to a *mut u8 is valid. + let ptr = value as *mut ManuallyDrop as *mut u8; // SAFETY: value is non null since it was initialized from an &mut above #[allow(unsafe_code)] - let ptr = unsafe { NonNull::new_unchecked(value) }; + let ptr = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: // * ptr was created from a u8, so is valid, aligned, and has correct provenance. // * ptr was created from an exclusive reference, so nothing else can read or write the value @@ -155,7 +161,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { // * values was initialized above, so references are valid #[allow(unsafe_code)] unsafe { - entity.insert_by_ids(&components, values.into_iter()); + entity.insert_by_ids(&components, ptrs.into_iter()); } } From a0b27e24c3a4f435fbe3cbe71c1d8d0b537487d7 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Fri, 20 Dec 2024 12:27:45 -0800 Subject: [PATCH 11/13] clean up comments some more --- examples/stress_tests/many_components.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 33be7ef04ea2c..18db9d467a4fc 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -133,8 +133,9 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { .collect(); let mut entity = world.spawn_empty(); - // We need to avoid dropping the u8's when `values` is dropped since ownership of the values - // is passed to the world. But we do want to deallocate the memory. + // We use `ManuallyDrop` here as we need to avoid dropping the u8's when `values` is dropped + // since ownership of the values is passed to the world in `insert_by_ids`. + // But we do want to deallocate the memory when values is dropped. let mut values: Vec> = components .iter() .map(|_id| ManuallyDrop::new(rng.gen_range(0..255))) @@ -148,8 +149,9 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { #[allow(unsafe_code)] let ptr = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: - // * ptr was created from a u8, so is valid, aligned, and has correct provenance. - // * ptr was created from an exclusive reference, so nothing else can read or write the value + // * values is valid until the end of the for block and this pointer is consumed by `insert_by_ids`. + // * ptr was created from a u8, so is aligned, and has correct provenance. + // * ptr was created from an exclusive reference and nothing else reads or writes to `values`. #[allow(unsafe_code)] unsafe { OwningPtr::new(ptr) @@ -157,8 +159,8 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { }) .collect(); // SAFETY: - // * component_id is from the same world - // * values was initialized above, so references are valid + // * component_id's are from the same world + // * `values` was initialized above, so references are valid #[allow(unsafe_code)] unsafe { entity.insert_by_ids(&components, ptrs.into_iter()); From c40434a18292a61d5d85b49c426d31128ac1938c Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Mon, 23 Dec 2024 22:19:50 -0800 Subject: [PATCH 12/13] fmt --- examples/stress_tests/many_components.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 18db9d467a4fc..12493ebba5c32 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -134,7 +134,7 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { let mut entity = world.spawn_empty(); // We use `ManuallyDrop` here as we need to avoid dropping the u8's when `values` is dropped - // since ownership of the values is passed to the world in `insert_by_ids`. + // since ownership of the values is passed to the world in `insert_by_ids`. // But we do want to deallocate the memory when values is dropped. let mut values: Vec> = components .iter() From d95764ba99e416ccaec4a21a9b67e7940e831342 Mon Sep 17 00:00:00 2001 From: Mike Hsu Date: Mon, 23 Dec 2024 22:45:40 -0800 Subject: [PATCH 13/13] use PtrMut::promote instead to preserve the lifetimes --- examples/stress_tests/many_components.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/examples/stress_tests/many_components.rs b/examples/stress_tests/many_components.rs index 12493ebba5c32..94f6352df02d3 100644 --- a/examples/stress_tests/many_components.rs +++ b/examples/stress_tests/many_components.rs @@ -23,13 +23,13 @@ use bevy::{ }, log::LogPlugin, prelude::{App, In, IntoSystem, Query, Schedule, SystemParamBuilder, Update}, - ptr::OwningPtr, + ptr::{OwningPtr, PtrMut}, MinimalPlugins, }; use rand::prelude::{Rng, SeedableRng, SliceRandom}; use rand_chacha::ChaCha8Rng; -use std::{alloc::Layout, mem::ManuallyDrop, num::Wrapping, ptr::NonNull}; +use std::{alloc::Layout, mem::ManuallyDrop, num::Wrapping}; // A simple system that matches against several components and does some menial calculation to create // some non-trivial load. @@ -143,18 +143,12 @@ fn stress_test(num_entities: u32, num_components: u32, num_systems: u32) { let ptrs: Vec = values .iter_mut() .map(|value| { - // ManuallyDrop is repr(transparent) so casting it to a *mut u8 is valid. - let ptr = value as *mut ManuallyDrop as *mut u8; - // SAFETY: value is non null since it was initialized from an &mut above - #[allow(unsafe_code)] - let ptr = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: - // * values is valid until the end of the for block and this pointer is consumed by `insert_by_ids`. - // * ptr was created from a u8, so is aligned, and has correct provenance. - // * ptr was created from an exclusive reference and nothing else reads or writes to `values`. + // * We don't read/write `values` binding after this and values are `ManuallyDrop`, + // so we have the right to drop/move the values #[allow(unsafe_code)] unsafe { - OwningPtr::new(ptr) + PtrMut::from(value).promote() } }) .collect();