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

Finer control over the DefaultTaskPoolOptions #2163

Closed
ElliotB256 opened this issue May 14, 2021 · 3 comments
Closed

Finer control over the DefaultTaskPoolOptions #2163

ElliotB256 opened this issue May 14, 2021 · 3 comments
Labels
A-Core Common functionality for all bevy apps C-Feature A new feature, making something new possible

Comments

@ElliotB256
Copy link

What problem does this solve or what need does it fill?

I would like finer control over the thread pool and I'm not sure how to modify the DefaultTaskPoolOptions to achieve this.
Following the examples, it seems I can use some methods to create a DefaultTaskPoolOptions, e.g.:

DefaultTaskPoolOptions::with_num_threads(4)

but this doesn't provide enough flexibility to e.g. disable the io pool or change the ratio of async_compute to compute threads.

I can create a DefaultTaskPoolOptions struct, but assigning values to some of the fields is not possible.
The task_pool_options mod in the core lib is not marked as pub, which prevents defining e.g. a TaskPoolThreadAssignmentPolicy.
Ideally, I would do something like:

    let mut pool = DefaultTaskPoolOptions::default();
    pool.io = bevy::core::task_pool_options::TaskPoolThreadAssignmentPolicy { min_threads: 0, max_threads: 2, percent: 0.2};

What solution would you like?

Define the task_pool_options mod as pub to allow construction of a DefaultTaskPoolOptions with all fields defined.

What alternative(s) have you considered?

Alternatively, there could be more methods for the DefaultTaskPoolOptions which modifies the compute, async_compute and io pools. Perhaps a DefaultTaskPoolBuilder could provide a way to configure a DefaultTaskPoolOptions?

@ElliotB256 ElliotB256 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 14, 2021
@alice-i-cecile alice-i-cecile added A-Core Common functionality for all bevy apps and removed S-Needs-Triage This issue needs to be labelled labels May 14, 2021
@TanTanDev
Copy link

I agree!
I'm currently only utilizing an AsyncComputeTaskPool, and I would greatly benefit if I could increase the threads it's using.
Since my computer only allows for 4 threads, I only ever get to utilize 1 thread since AsyncComputeTaskPool is assigned 25% threads.

Opening up customization would greatly benefit my project I believe.

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 4, 2022

All fields are already public. DefaultTaskPoolOptions is also exported so you can do something like pool.io.min_threads = 0;. I do agree that TaskPoolThreadAssignmentPolicy should be exported too.

@TanTanDev
Copy link

II see, so this is what we currently have to do to modify the DefaultTaskPoolOptions

    let mut task_pool_settings = DefaultTaskPoolOptions::default();
    task_pool_settings.async_compute.percent = 1.0f32;
    task_pool_settings.compute.percent = 0.0f32; // i currently only use async_compute
    task_pool_settings.io.percent = 0.0f32; // always use 1
    .... app setup code
    .insert_resource(task_pool_settings)

I don't think that is the most 'elegant' way to write it, if the TaskPoolThreadAssignmentPolicy was pub in the namespace
We would then be able to write it this way:

        .insert_resource(DefaultTaskPoolOptions{
            async_compute: TaskPoolThreadAssignmentPolicy{
                min_threads: 1,
                max_threads: 4,
                percent: 0.25,
            },
            ..Default::default()
        })

I guess we can get around it by doing the first method, but I feel like the second way is more what I would expect.

@bors bors bot closed this as completed in de2a47c Mar 8, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this issue Jun 7, 2022
# Objective

- Fix bevyengine#2163
- Allow configuration of thread pools through `DefaultTaskPoolOptions`

## Solution

- `TaskPoolThreadAssignmentPolicy` was already public but not exported. Export it.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Fix bevyengine#2163
- Allow configuration of thread pools through `DefaultTaskPoolOptions`

## Solution

- `TaskPoolThreadAssignmentPolicy` was already public but not exported. Export it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Core Common functionality for all bevy apps C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants