-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Optimize Event Updates #12936
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize Event Updates #12936
Changes from all commits
65449ea
dd93284
59a3d3c
4a6e196
5a9f98a
b9830cb
f790cd2
c53777b
9759e9c
c906686
a5a8271
908003f
d726f5e
60e2cf3
d4b74a1
6f160a8
c53bca5
836ade6
99dca64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,13 @@ | ||
//! Event handling types. | ||
|
||
use crate as bevy_ecs; | ||
use crate::system::{Local, Res, ResMut, Resource, SystemParam}; | ||
use crate::change_detection::MutUntyped; | ||
use crate::{ | ||
change_detection::{DetectChangesMut, Mut}, | ||
component::{ComponentId, Tick}, | ||
system::{Local, Res, ResMut, Resource, SystemParam}, | ||
world::World, | ||
}; | ||
pub use bevy_ecs_macros::Event; | ||
use bevy_ecs_macros::SystemSet; | ||
use bevy_utils::detailed_trace; | ||
|
@@ -253,7 +259,13 @@ impl<E: Event> Events<E> { | |
/// | ||
/// If you need access to the events that were removed, consider using [`Events::update_drain`]. | ||
pub fn update(&mut self) { | ||
let _ = self.update_drain(); | ||
std::mem::swap(&mut self.events_a, &mut self.events_b); | ||
self.events_b.clear(); | ||
self.events_b.start_event_count = self.event_count; | ||
debug_assert_eq!( | ||
self.events_a.start_event_count + self.events_a.len(), | ||
self.events_b.start_event_count | ||
); | ||
} | ||
|
||
/// Swaps the event buffers and drains the oldest event buffer, returning an iterator | ||
|
@@ -798,48 +810,90 @@ impl<'a, E: Event> ExactSizeIterator for EventIteratorWithId<'a, E> { | |
} | ||
} | ||
|
||
#[doc(hidden)] | ||
struct RegisteredEvent { | ||
component_id: ComponentId, | ||
// Required to flush the secondary buffer and drop events even if left unchanged. | ||
previously_updated: bool, | ||
// SAFETY: The component ID and the function must be used to fetch the Events<T> resource | ||
// of the same type initialized in `register_event`, or improper type casts will occur. | ||
update: unsafe fn(MutUntyped), | ||
} | ||
|
||
/// A registry of all of the [`Events`] in the [`World`], used by [`event_update_system`] | ||
/// to update all of the events. | ||
#[derive(Resource, Default)] | ||
pub struct EventUpdateSignal(bool); | ||
pub struct EventRegistry { | ||
needs_update: bool, | ||
event_updates: Vec<RegisteredEvent>, | ||
} | ||
|
||
impl EventRegistry { | ||
/// Registers an event type to be updated. | ||
pub fn register_event<T: Event>(world: &mut World) { | ||
// By initializing the resource here, we can be sure that it is present, | ||
// and receive the correct, up-to-date `ComponentId` even if it was previously removed. | ||
let component_id = world.init_resource::<Events<T>>(); | ||
james7132 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let mut registry = world.get_resource_or_insert_with(Self::default); | ||
registry.event_updates.push(RegisteredEvent { | ||
component_id, | ||
previously_updated: false, | ||
update: |ptr| { | ||
// SAFETY: The resource was initialized with the type Events<T>. | ||
unsafe { ptr.with_type::<Events<T>>() } | ||
.bypass_change_detection() | ||
.update(); | ||
}, | ||
}); | ||
} | ||
|
||
/// Updates all of the registered events in the World. | ||
pub fn run_updates(&mut self, world: &mut World, last_change_tick: Tick) { | ||
for registered_event in &mut self.event_updates { | ||
// Bypass the type ID -> Component ID lookup with the cached component ID. | ||
if let Some(events) = world.get_resource_mut_by_id(registered_event.component_id) { | ||
let has_changed = events.has_changed_since(last_change_tick); | ||
if registered_event.previously_updated || has_changed { | ||
// SAFETY: The update function pointer is called with the resource | ||
// fetched from the same component ID. | ||
unsafe { (registered_event.update)(events) }; | ||
// Always set to true if the events have changed, otherwise disable running on the second invocation | ||
// to wait for more changes. | ||
registered_event.previously_updated = | ||
has_changed || !registered_event.previously_updated; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
#[doc(hidden)] | ||
#[derive(SystemSet, Clone, Debug, PartialEq, Eq, Hash)] | ||
pub struct EventUpdates; | ||
|
||
/// Signals the [`event_update_system`] to run after `FixedUpdate` systems. | ||
pub fn signal_event_update_system(signal: Option<ResMut<EventUpdateSignal>>) { | ||
if let Some(mut s) = signal { | ||
s.0 = true; | ||
pub fn signal_event_update_system(signal: Option<ResMut<EventRegistry>>) { | ||
if let Some(mut registry) = signal { | ||
registry.needs_update = true; | ||
} | ||
} | ||
|
||
/// Resets the `EventUpdateSignal` | ||
pub fn reset_event_update_signal_system(signal: Option<ResMut<EventUpdateSignal>>) { | ||
if let Some(mut s) = signal { | ||
s.0 = false; | ||
} | ||
} | ||
|
||
/// A system that calls [`Events::update`]. | ||
pub fn event_update_system<T: Event>( | ||
update_signal: Option<Res<EventUpdateSignal>>, | ||
mut events: ResMut<Events<T>>, | ||
) { | ||
if let Some(signal) = update_signal { | ||
// If we haven't got a signal to update the events, but we *could* get such a signal | ||
// return early and update the events later. | ||
if !signal.0 { | ||
return; | ||
} | ||
/// A system that calls [`Events::update`] on all registered [`Events`] in the world. | ||
pub fn event_update_system(world: &mut World, mut last_change_tick: Local<Tick>) { | ||
if world.contains_resource::<EventRegistry>() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this better than just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If for whatever reason, someone is running without any events in their World, this would avoid stealth allocating it. |
||
world.resource_scope(|world, mut registry: Mut<EventRegistry>| { | ||
registry.run_updates(world, *last_change_tick); | ||
// Disable the system until signal_event_update_system runs again. | ||
registry.needs_update = false; | ||
}); | ||
} | ||
|
||
events.update(); | ||
*last_change_tick = world.change_tick(); | ||
} | ||
|
||
/// A run condition that checks if the event's [`event_update_system`] | ||
/// needs to run or not. | ||
pub fn event_update_condition<T: Event>(events: Res<Events<T>>) -> bool { | ||
!events.events_a.is_empty() || !events.events_b.is_empty() | ||
/// A run condition for [`event_update_system`]. | ||
pub fn event_update_condition(signal: Option<Res<EventRegistry>>) -> bool { | ||
// If we haven't got a signal to update the events, but we *could* get such a signal | ||
// return early and update the events later. | ||
signal.map_or(false, |signal| signal.needs_update) | ||
} | ||
|
||
/// [`Iterator`] over sent [`EventIds`](`EventId`) from a batch. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
use bevy_ecs::system::{Res, ResMut}; | ||
use bevy_reflect::Reflect; | ||
use bevy_utils::{tracing::debug, Duration}; | ||
|
||
|
@@ -268,11 +267,7 @@ impl Default for Virtual { | |
/// Advances [`Time<Virtual>`] and [`Time`] based on the elapsed [`Time<Real>`]. | ||
/// | ||
/// The virtual time will be advanced up to the provided [`Time::max_delta`]. | ||
pub fn virtual_time_system( | ||
mut current: ResMut<Time>, | ||
mut virt: ResMut<Time<Virtual>>, | ||
real: Res<Time<Real>>, | ||
) { | ||
pub fn update_virtual_time(current: &mut Time, virt: &mut Time<Virtual>, real: &Time<Real>) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the bevy_time changes supposed to be part of this OR? I don't understand how they're related. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in a similar vein of being a common cost in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a moderate preference to move these changes into their own PR, but won't block on it. |
||
let raw_delta = real.delta(); | ||
virt.advance_with_raw_delta(raw_delta); | ||
*current = virt.as_generic(); | ||
|
Uh oh!
There was an error while loading. Please reload this page.