Task System for Bevy #384

lachlansneff · 2020-08-28T17:12:09Z

Currently bevy depends on rayon for multithreaded dispatching of systems. This PR replaces rayon with a custom designed task system that consists of several threaded "TaskPools."

Why replace Rayon

Rayon has long-standing performance issues with smaller workloads on machines with more than a few cores. (cpu usage #111)
Rayon is not async-friendly
Rayon is somewhat of a closed box, it owns the threads and it would be more difficult to upstream changes that are tuned for games as it’s a general purpose library
Rayon has a lot more in it than we really need. Our alternative has fewer dependencies, compiles faster, and is substantially less code

Bevy Tasks

bevy_tasks does three things:

Fire-and-forget `static tasks
Fork-join for non-static tasks
Chunked parallel iteration of slices

These are enough to replace all current rayon functionality within bevy, as well as to provide additional functionality that can partially supplant parts of rayon's parallel iteration support.

Exposed API

Exports IOTaskPool, ComputePool, and AsyncComputePool.
bevy_app exports DefaultTaskPoolOptions now.
Documentation mirrored at https://charted.space/notes/bevy_tasks/bevy_tasks/index.html.

Future work:

Continue distributing more work onto task pools (e.g. asset handlers).
More optimization of cpu usage.

Resolves #318

Move the github.com/lachlansneff/small-tasks prototype into bevy/crates/bevy_tasks. This is intended to replace rayon to provide better idling behavior. Initial commit Make scope return a vector of results Add par_chunk_map(_mut) and par_splat_map(_mut) apis onto slices Make calling thread do work until complete Remove current-thread doing work Ignore clion/intellij project files Remove dependency on futures-util Set up a builder for constructing the TaskPool with customizable thread count, stack size, and thread name. Add a couple examples to demonstrate busy and idle workloads. More detail on thread name doc comments and static build() function on TaskPool Update to README (add description, status, license) Reflect rayon's ThreadPoolBuilder api in TaskPoolBuilder Co-authored-by: Philip Degarmo <aclysma@gmail.com>

Add test of global pool Formatting

… already installed globally

…or (#1) Add a task newtype so that we don't expose multitask::Task Remove some of the README/license info that was present in the prototype repo Add some doc comments

…-tasks

…asks

…l a resource instead of an internal static

* Remove generics from TaskPool newtypes and some of the task API Add IO, AsyncCompute and Compute TaskPools Move TaskPool setup from bevy_ecs to bevy_app ParallelExecutorOptions is essentially replaced by DefaultTaskPoolOptions * Pull TaskPool and related types out of bevy_tasks/lib.rs into a separate module Add a prelude to bevy_tasks Update the version of bevy_tasks to match other crates * Assert percent of cores >= 0

bgourlie · 2020-08-28T18:07:23Z

Our alternative... compiles faster

Nice! Any idea by how much?

cart · 2020-08-28T21:06:40Z

Just to crudely illustrate how much of an improvement this is, when I run the breakout example I get the following cpu usage (average across cores):

rayon 1.3: ~50%
rayon 1.4: ~40%
bevy_tasks: ~20%

cart · 2020-08-28T21:15:53Z

This also cuts our dependency count by 7 and significantly decreases compile times.

For example, before breakout clean-compiled in release mode on my machine in 2 minutes 3 seconds

Now it compiles in 1 minute 34 seconds!

cart · 2020-08-28T21:25:50Z

Actually I ran a few more tests and I dont think the difference is quite that stark. Probably closer to 5-10 seconds.

kabergstrom · 2020-08-28T21:38:14Z

Great stuff!
I think it would be very cool to be able to spawn tasks on a thread-local executor too. Sometimes you have things that are !Send, and you can then still use them in an async context by wrapping them in an Rc or something. It should be quite easy to support:

Create a thread-local var like this:

thread_local! {
    static EXECUTOR: RefCell<LocalExecutor> = RefCell::new(LocalExecutor::new());
}

Each worker thread polls its LocalExecutor in addition to its Ticker within its loop.
Expose a public function called spawn_local like this: https://github.com/async-rs/async-std/blob/master/src/task/executor.rs#L19

To protect from people trying to spawn_local on a non-worker thread, may want the thread_local to be an Option<RefCell<LocalExecutor>>?

cart

Fantastic work. A big thanks to everyone involved!

cart · 2020-08-28T23:14:04Z

crates/bevy_app/Cargo.toml

@@ -13,6 +13,8 @@ keywords = ["bevy"]
 # bevy
 bevy_derive = { path = "../bevy_derive", version = "0.1" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.1" }
+bevy_tasks = { path = "../bevy_tasks" }
+num_cpus = "1"


bevy_tasks also relies on num_cpus. this seems like functionality that is relevant enough to consumers that we could just provide a bevy_tasks api.

Sounds good

cart · 2020-08-28T23:17:01Z

crates/bevy_app/src/app.rs

@@ -63,6 +63,12 @@ impl App {
    }

    pub fn run(mut self) {
+        // Setup the default bevy task pools
+        self.resources


I'm not convinced bevy_tasks should be hard-coded into apps, especially given that we're just adding resources. Can we make this a plugin that we add with add_default_plugins()?

Making a plugin in bevy_tasks will cause circular dependencies (app depends on ecs, ecs depends on tasks). Where to do you want it to be?

cart · 2020-08-28T23:17:57Z

crates/bevy_app/src/task_pool_options.rs

+use bevy_ecs::Resources;
+use bevy_tasks::{AsyncComputeTaskPool, ComputeTaskPool, IOTaskPool, TaskPoolBuilder};
+
+fn clamp_usize(value: usize, min: usize, max: usize) -> usize {


Clamp is a big enough missing piece in the rust std that it probably makes sense to expose it in bevy_math. Can we add a generic version there and then consume it here?

Ok, I'll copy/paste num-traits implementation (licensed MIT) with a note as to where it came from

https://doc.rust-lang.org/std/cmp/trait.Ord.html#method.clamp

it's available on nightly, is it possible to switch to the default impl for nightly users?

Ooh I didn't know that. Definitely good to know. In general I would prefer to wait for nightly features to land in stable. Otherwise we'll need to maintain multiple code paths, which makes it harder to debug and repro potential issues.

crates/bevy_app/src/task_pool_options.rs

crates/bevy_tasks/README.md

cart · 2020-08-28T23:31:18Z

crates/bevy_tasks/src/slice.rs

+        let task_pool = TaskPool::new();
+        let outputs = v.par_splat_map(&task_pool, None, |numbers| -> i32 { numbers.iter().sum() });
+
+        println!("outputs: {:?}", outputs);


can we swap the println!() calls in these tests for asserts on output values?

cart · 2020-08-28T23:31:26Z

crates/bevy_tasks/src/task_pool.rs

+    /// Vec<Task<T>> contained within TaskPoolInner
+    executor: Arc<multitask::Executor>,
+
+    ///


nit: empty docs

crates/bevy_tasks/src/task_pool.rs

cart · 2020-08-28T23:43:33Z

@kabergstrom yeah that would be a welcome feature. when this gets merged, can you create an issue for it? id prefer to keep this pr constrained to its current scope.

@lachlansneff some of the schedule tests are failing because ComputeTaskPool doesn't exist. we'll need to fix them before merging.

aclysma · 2020-08-29T06:00:54Z

Just to crudely illustrate how much of an improvement this is, when I run the breakout example I get the following cpu usage (average across cores):

rayon 1.3: ~50%
rayon 1.4: ~40%
bevy_tasks: ~20%

On a 3950x with 32 cores, I see a bigger difference (and rayon 1.3 and 1.4 give the same results for me):
Debug mode: 900% -> 110%
Release mode: 450% -> 40%

aclysma · 2020-08-29T08:26:28Z

I think I addressed everything - I PR'ed it to @lachlansneff's branch so he can review it before we ask @cart to take another look.

* Address some feedback for the bevy_tasks PR - Add a general clamp fn to bevy_math - Expose num_cpus in bevy_tasks - Fill out empty doc comment * Add comments and clean up pinning in bevy_tasks, fix a couple tests

Cargo.toml

crates/bevy_app/Cargo.toml

Co-authored-by: stefee <srilq.git@gmail.com>

aclysma · 2020-08-29T18:27:54Z

@cart I think this is ready for a second look, the two main issues remaining are:

“I'm not convinced bevy_tasks should be hard-coded into apps...”
“Ordering determining "precedence" is maybe a little hard to reason about...“

cart · 2020-08-29T19:35:15Z

Ok I think this ready to merge now. The "bevy_tasks hard coded into apps" issue can be resolved separately. I'd rather merge this and then iterate than wait longer.

One more huge thanks to everyone involved. This is a big step forward!

Add bevy_tasks crate to replace rayon

I chose to remove the extended documentation for `TaskPool::executor`. It references `TaskPoolInner` which was added in bevyengine#384 when `bevy_tasks` was first created, but that struct was later removed in bevyengine#2250. James may be able to rewrite the comment, but I do not know enough about the task pool to write anything.

lachlansneff and others added 14 commits August 23, 2020 16:30

More tuning of bevy_tasks api

fe6cf0d

Add global task pool to bevy_tasks

058e26c

Add test of global pool Formatting

Replace uses of rayon with bevy_tasks

d6cbe59

Fix leaked task_pool when attempting to install while a task pool was…

1a55abe

… already installed globally

Fix typo

c76cb5f

Add spawn() public function to pass raw futures through to the execut…

cd7ce62

…or (#1) Add a task newtype so that we don't expose multitask::Task Remove some of the README/license info that was present in the prototype repo Add some doc comments

Merge branch 'master' of https://github.com/bevyengine/bevy into bevy…

3014032

…-tasks

Merge branch 'bevy-tasks' of github.com:lachlansneff/bevy into bevy-t…

4b3619f

…asks

Update bevy_tasks with new Usage type parameter and make the task poo…

4232e75

…l a resource instead of an internal static

Add to root Cargo.toml and lib.rs

93fcd7d

Fix tests

5a59d62

Fix thread_pool_resources example

d092add

cart reviewed Aug 28, 2020

View reviewed changes

aclysma added 2 commits August 29, 2020 11:34

Bevy tasks (#3)

8996109

* Address some feedback for the bevy_tasks PR - Add a general clamp fn to bevy_math - Expose num_cpus in bevy_tasks - Fill out empty doc comment * Add comments and clean up pinning in bevy_tasks, fix a couple tests

Merge branch 'master' into bevy-tasks

224f358

stefee reviewed Aug 29, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

crates/bevy_app/Cargo.toml Outdated Show resolved Hide resolved

aclysma and others added 2 commits August 29, 2020 11:01

Fix tests

879afa9

Add version to bevy_tasks references in cargo.toml files

77b126d

Co-authored-by: stefee <srilq.git@gmail.com>

cart merged commit 17e7642 into bevyengine:master Aug 29, 2020

CleanCut mentioned this pull request Aug 29, 2020

cpu usage #111

Closed

utherpally mentioned this pull request Aug 30, 2020

testing samples: rendering to device is not working #366

Closed

Moxinilian added C-Enhancement A new feature C-Performance A change motivated by improving speed, memory usage or compile times labels Aug 30, 2020

sunwukonga mentioned this pull request Aug 31, 2020

Breakout example crashes when limited to a single CPU #405

Closed

mrk-its pushed a commit to mrk-its/bevy that referenced this pull request Oct 6, 2020

Task System for Bevy (bevyengine#384)

ada462c

Add bevy_tasks crate to replace rayon

re0312 mentioned this pull request Jan 23, 2024

Rayon's Minimal Parallel Infrastructure With Bevy Task #11492

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task System for Bevy #384

Task System for Bevy #384

lachlansneff commented Aug 28, 2020 •

edited

Loading

bgourlie commented Aug 28, 2020 •

edited

Loading

cart commented Aug 28, 2020

cart commented Aug 28, 2020

cart commented Aug 28, 2020

kabergstrom commented Aug 28, 2020

cart left a comment

cart Aug 28, 2020

aclysma Aug 29, 2020

cart Aug 28, 2020

aclysma Aug 29, 2020 •

edited

Loading

cart Aug 28, 2020

aclysma Aug 29, 2020

berkus Sep 8, 2020

cart Sep 8, 2020

cart Aug 28, 2020

aclysma Aug 29, 2020

cart Aug 28, 2020

aclysma Aug 29, 2020

cart commented Aug 28, 2020

aclysma commented Aug 29, 2020

aclysma commented Aug 29, 2020

aclysma commented Aug 29, 2020

cart commented Aug 29, 2020

Task System for Bevy #384

Task System for Bevy #384

Conversation

lachlansneff commented Aug 28, 2020 • edited Loading

Why replace Rayon

Bevy Tasks

Exposed API

bgourlie commented Aug 28, 2020 • edited Loading

cart commented Aug 28, 2020

cart commented Aug 28, 2020

cart commented Aug 28, 2020

kabergstrom commented Aug 28, 2020

cart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aclysma Aug 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cart commented Aug 28, 2020

aclysma commented Aug 29, 2020

aclysma commented Aug 29, 2020

aclysma commented Aug 29, 2020

cart commented Aug 29, 2020

lachlansneff commented Aug 28, 2020 •

edited

Loading

bgourlie commented Aug 28, 2020 •

edited

Loading

aclysma Aug 29, 2020 •

edited

Loading