Specify thread pool by default for the parallel iteration of queries #3183
Labels
A-ECS
Entities, components, systems, and events
C-Usability
A targeted quality-of-life change that makes Bevy easier to use
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
Comments
alice-i-cecile
added
A-ECS
alice-i-cecile
added
the
S-Ready-For-Implementation
This was referenced May 26, 2022
james7132
added a commit
to james7132/bevy
that referenced
this issue
Jun 7, 2022
# Objective Fixes bevyengine#3183. Requiring a `&TaskPool` parameter is sort of meaningless if the only correct one is to use the one provided by `Res<ComputeTaskPool>` all the time. ## Solution Have `QueryState` save a clone of the `ComputeTaskPool` which is used for all `par_for_each` functions. ~~Adds a small overhead of the internal `Arc` clone as a part of the startup, but the ergonomics win should be well worth this hardly-noticable overhead.~~ Updated the docs to note that it will panic the task pool is not present as a resource. # Future Work If bevyengine/rfcs#54 is approved, we can replace these resource lookups with a static function call instead to get the `ComputeTaskPool`. --- ## Changelog Removed: The `task_pool` parameter of `Query(State)::par_for_each(_mut)`. These calls will use the `World`'s `ComputeTaskPool` resource instead. ## Migration Guide The `task_pool` parameter for `Query(State)::par_for_each(_mut)` has been removed. Remove these parameters from all calls to these functions. Before: ```rust fn parallel_system( task_pool: Res<ComputeTaskPool>, query: Query<&MyComponent>, ) { query.par_for_each(&task_pool, 32, |comp| { ... }); } ``` After: ```rust fn parallel_system(query: Query<&MyComponent>) { query.par_for_each(32, |comp| { ... }); } ``` If using `Query(State)` outside of a system run by the scheduler, you may need to manually configure and initialize a `ComputeTaskPool` as a resource in the `World`.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this issue
Feb 1, 2023
# Objective Fixes bevyengine#3183. Requiring a `&TaskPool` parameter is sort of meaningless if the only correct one is to use the one provided by `Res<ComputeTaskPool>` all the time. ## Solution Have `QueryState` save a clone of the `ComputeTaskPool` which is used for all `par_for_each` functions. ~~Adds a small overhead of the internal `Arc` clone as a part of the startup, but the ergonomics win should be well worth this hardly-noticable overhead.~~ Updated the docs to note that it will panic the task pool is not present as a resource. # Future Work If bevyengine/rfcs#54 is approved, we can replace these resource lookups with a static function call instead to get the `ComputeTaskPool`. --- ## Changelog Removed: The `task_pool` parameter of `Query(State)::par_for_each(_mut)`. These calls will use the `World`'s `ComputeTaskPool` resource instead. ## Migration Guide The `task_pool` parameter for `Query(State)::par_for_each(_mut)` has been removed. Remove these parameters from all calls to these functions. Before: ```rust fn parallel_system( task_pool: Res<ComputeTaskPool>, query: Query<&MyComponent>, ) { query.par_for_each(&task_pool, 32, |comp| { ... }); } ``` After: ```rust fn parallel_system(query: Query<&MyComponent>) { query.par_for_each(32, |comp| { ... }); } ``` If using `Query(State)` outside of a system run by the scheduler, you may need to manually configure and initialize a `ComputeTaskPool` as a resource in the `World`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
The existing parallel iteration strategy requires specifying a thread pool, generally through
Res<ComputeTaskPool>, and then passing in the parameters via a closure. See https://github.com/bevyengine/bevy/blob/main/examples/ecs/parallel_query.rsWhat solution would you like?
ComputeTaskPoolresource with each query. This is cloned during system initialization.par_for_each_manualand so on.par_for_each(and eventuallypar_iterand friends) which uses this task pool.What alternative(s) have you considered?
We could instead create a global / static thread pool that is always used. This may be necessary for performance or ergonomics reasons, but should not be necessary.
Additional context
We cannot deprecate the existing techniques, as other thread pools may be desired in niche situations.
Related to #2088.
Discussed with @cart on Discord.
The text was updated successfully, but these errors were encountered: