Skip to content
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

Parallel event reader #12554

Merged
merged 14 commits into from
Apr 22, 2024
Merged

Conversation

yyogo
Copy link
Contributor

@yyogo yyogo commented Mar 18, 2024

Objective

Allow parallel iteration over events, resolve #10766

Solution

  • Add EventParIter which works similarly to QueryParIter, implementing a for_each{_with_id} operator.
    I chose to not mirror EventIteratorWithId and instead implement both operations on a single struct.
  • Reuse BatchingStrategy from QueryParIter

Changelog

  • EventReader now supports parallel event iteration using par_read().for_each(|event| ...).

Implement `EventReader::par_read()` returning `EventParIter`.
Currently the only operator implemented is `for_each`.
Comment on lines +441 to +470
/// ```
/// # use bevy_ecs::prelude::*;
/// # use std::sync::atomic::{AtomicUsize, Ordering};
///
/// #[derive(Event)]
/// struct MyEvent {
/// value: usize,
/// }
///
/// #[derive(Resource, Default)]
/// struct Counter(AtomicUsize);
///
/// // setup
/// let mut world = World::new();
/// world.init_resource::<Events<MyEvent>>();
/// world.insert_resource(Counter::default());
///
/// let mut schedule = Schedule::default();
/// schedule.add_systems(|mut events: EventReader<MyEvent>, counter: Res<Counter>| {
/// events.par_read().for_each(|MyEvent { value }| {
/// counter.0.fetch_add(*value, Ordering::Relaxed);
/// });
/// });
/// for value in 0..100 {
/// world.send_event(MyEvent { value });
/// }
/// schedule.run(&mut world);
/// let Counter(counter) = world.remove_resource::<Counter>().unwrap();
/// // all events were processed
/// assert_eq!(counter.into_inner(), 4950);
Copy link
Contributor Author

@yyogo yyogo Mar 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example is a bit contrived

@yyogo yyogo marked this pull request as ready for review March 18, 2024 16:45
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good API surface. Ideally we wouldn't need to be manually batching, and employ something similar to Rayon's workstealing in more flexible slices, but that may be a while before we can tackle something like that.

crates/bevy_ecs/src/event.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/event.rs Show resolved Hide resolved
crates/bevy_ecs/src/event.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/event.rs Outdated Show resolved Hide resolved
@james7132 james7132 added C-Enhancement A new feature A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times labels Mar 18, 2024
Also document that event IDs are increasing by send order.
This is already implied due to the `Ord` trait impl.
@yyogo yyogo marked this pull request as draft March 19, 2024 08:40
Encapsulate the batch size calculation logic in `BatchingStrategy`
@yyogo yyogo changed the title Parallel event iteration support Parallel event reader Mar 19, 2024
@yyogo yyogo marked this pull request as ready for review March 19, 2024 09:28
@yyogo yyogo requested a review from james7132 March 19, 2024 09:28
Copy link
Contributor

@pablo-lua pablo-lua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some comments in docs, otherwise, all good.

crates/bevy_ecs/src/batching.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/batching.rs Outdated Show resolved Hide resolved
@pablo-lua pablo-lua added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Apr 16, 2024
yyogo and others added 2 commits April 16, 2024 16:24
Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 22, 2024
Merged via the queue into bevyengine:main with commit e9be54b Apr 22, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Enhancement A new feature C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support parallel iteration over events
4 participants