Global TaskPool API improvements #10008
Conversation
|
This PR was originally also going to fix #9477, but I couldn't figure out how best to implement it, and I figure it would probably be better to split that out from these other improvements anyhow. |
| @@ -20,7 +20,7 @@ pub fn heavy_compute(c: &mut Criterion) { | |||
| group.warm_up_time(std::time::Duration::from_millis(500)); | |||
| group.measurement_time(std::time::Duration::from_secs(4)); | |||
| group.bench_function("base", |b| { | |||
| ComputeTaskPool::init(TaskPool::default); | |||
| ComputeTaskPool::get_or_init(TaskPool::default); | |||
There was a problem hiding this comment.
Half of the calls are with the default value, maybe add a .get_or_init_default method?
There was a problem hiding this comment.
My intuition is that a lot of these calls could just be replaced with calls to the panicking getter, but you would have to look at the context of each call to be sure, which is probably out of the scope of this PR.
Regardless, I don't think that a default initializer should be added, as TaskPoolOptions from bevy_core is the preferred way of initializing the global taskpools. We should be encouraging using that, and discouraging using TaskPool::default.
TaskPoolOptions could potentially be moved from bevy_core to bevy_tasks to facilitate using that instead of TaskPool::default, but that is definitely outside of the scope of this PR.
alice-i-cecile
added
C-Usability
|
Generally looks good. A bit reluctant to use macros for code deduplication purposes though. I'm not sure there's a better solution though: |
james7132
added
the
C-Breaking-Change
alice-i-cecile
added
the
S-Ready-For-Final-Review
# Objective Reduce code duplication and improve APIs of Bevy's [global taskpools](https://github.com/bevyengine/bevy/blob/main/crates/bevy_tasks/src/usages.rs). ## Solution - As all three of the global taskpools have identical implementations and only differ in their identifiers, this PR moves the implementation into a macro to reduce code duplication. - The `init` method is renamed to `get_or_init` to more accurately reflect what it really does. - Add a new `try_get` method that just returns `None` when the pool is uninitialized, to complement the other getter methods. - Minor documentation improvements to accompany the above changes. --- ## Changelog - Added a new `try_get` method to the global TaskPools - The global TaskPools' `init` method has been renamed to `get_or_init` for clarity - Documentation improvements ## Migration Guide - Uses of `ComputeTaskPool::init`, `AsyncComputeTaskPool::init` and `IoTaskPool::init` should be changed to `::get_or_init`.
# Objective Reduce code duplication and improve APIs of Bevy's [global taskpools](https://github.com/bevyengine/bevy/blob/main/crates/bevy_tasks/src/usages.rs). ## Solution - As all three of the global taskpools have identical implementations and only differ in their identifiers, this PR moves the implementation into a macro to reduce code duplication. - The `init` method is renamed to `get_or_init` to more accurately reflect what it really does. - Add a new `try_get` method that just returns `None` when the pool is uninitialized, to complement the other getter methods. - Minor documentation improvements to accompany the above changes. --- ## Changelog - Added a new `try_get` method to the global TaskPools - The global TaskPools' `init` method has been renamed to `get_or_init` for clarity - Documentation improvements ## Migration Guide - Uses of `ComputeTaskPool::init`, `AsyncComputeTaskPool::init` and `IoTaskPool::init` should be changed to `::get_or_init`.
Objective
Reduce code duplication and improve APIs of Bevy's global taskpools.
Solution
initmethod is renamed toget_or_initto more accurately reflect what it really does.try_getmethod that just returnsNonewhen the pool is uninitialized, to complement the other getter methods.Changelog
try_getmethod to the global TaskPoolsinitmethod has been renamed toget_or_initfor clarityMigration Guide
ComputeTaskPool::init,AsyncComputeTaskPool::initandIoTaskPool::initshould be changed to::get_or_init.