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

Example of custom async task with single block_on #2650

Closed
wants to merge 5 commits into from

Conversation

hanabi1224
Copy link

@hanabi1224 hanabi1224 commented Aug 14, 2021

Objective

  • Compare performance between using single block_on and multiple block_on(s) when dealing with massive custom async tasks
  • The result shows (on windows) that using single block_on has better performance (by 20%~50%), is it feasible to perform poll_once inside ecs framework then, that would save a lot of boilerplate code?
  • By avoiding poll_once and block_on in user code, it gets the best performance.

Solution

  • Add an async-bench example
  • Run with release build cargo run --release --example async_bench
  • It pints result like
[handle_tasks_no_poll_once]n_frames executed: 206, avg fps: 34.8(target:120), duration: 5.920s
[handle_tasks]        n_frames executed: 72, avg fps: 12.4(target:120), duration: 5.784s
[handle_tasks_par]    n_frames executed: 123, avg fps: 21.6(target:120), duration: 5.697s
[handle_tasks_par_2]  n_frames executed: 88, avg fps: 15.3(target:120), duration: 5.751s

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Aug 14, 2021
Copy link
Member

@NiklasEi NiklasEi left a comment

Choose a reason for hiding this comment

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

This seems to be more of a benchmark than an example. Can it be moved to benches?

@NiklasEi NiklasEi added A-Tasks Tools for parallel and async work and removed S-Needs-Triage This issue needs to be labelled labels Aug 16, 2021
@hanabi1224
Copy link
Author

@NiklasEi Thanks! Sure I can add another micro-benchmark to the benches, in the meanwhile, this one is more like a small toy program that exercises the main stacks of bevy ecs, and show numbers in an E2E scenario. I think micro-benchmark and small benchmark program serves different purposes and should co-exsist.

BTW, It does show a huge perf gap between using single and multiple block_on(s), regarding my question below, where is the best place for discussion? GH discussion or discord?

is it feasible to perform poll_once inside ecs framework then, that would save a lot of boilerplate code?

@mockersf
Copy link
Member

The difference between handle_tasks_par and handle_tasks_par_2 are due to the .collect::<Vec<_>>(). Also, you can squeeze a little more speed by changing your n_tasks to a bool, as you don't actually care about the number of task performed.

On the difference between multiple block_on vs a single bigger one, this is more a question for futures-lite but it makes sense as blocking the thread is expensive, so doing it only once is better. Discussion on GH discussion is best as answers will be easily searchable, don't hesitate to post your discussion link on discord if you don't get answers 👍

is it feasible to perform poll_once inside ecs framework then, that would save a lot of boilerplate code?

Are you suggesting something like

struct TaskResult<T>(T);

pub fn handle_tasks<T: Component>(
    mut commands: Commands,
    mut tasks: Query<(Entity, &mut Task<T>)>,
) {
    let done = tasks.iter_mut().map(|(entity, mut task)| async move {
        if let Some(r) = poll_once(&mut *task).await {
            Some((entity, r))
        } else {
            None
        }
    });
    block_on(async {
        for f in done {
            if let Some((entity, result)) = f.await {
                commands
                    .entity(entity)
                    .remove::<Task<T>>()
                    .insert(TaskResult(result));
            }
        }
    });
}

It's possible to add in Bevy, but as it needs to be generic over the task type, you would still need some kind of registration mechanic for your tasks

@hanabi1224
Copy link
Author

hanabi1224 commented Aug 18, 2021

@mockersf Thanks! I'm thinking of sth like this, users still register Task but can query PollOnce<Task<T>> directly
I guess this requires implementing SysParams for PollOnce<Task<T>> and perform poll_once in fetch after retrieving tasks from entity table

pub fn handle_tasks<T: Component>(
    mut commands: Commands,
    mut tasks: Query<(Entity, PollOnce<Task<T>>)>, 
) {
    block_on(async {
        for entity, task in tasks{
            if let Some(result) = task.await {
                commands
                    .entity(entity)
                    .remove::<Task<T>>()
                    .insert(TaskResult(result));
            }
        }
    });
}

Or, if async fn can be used, ecs will do the block_on part

pub async fn handle_tasks<T: Component>(
    mut commands: Commands,
    mut tasks: Query<(Entity, &mut Task<T>)>,
) {
    for (entity, mut task) in transform_tasks.iter_mut() {
        n_tasks += 1;
        let ret = poll_once(&mut *task).await;
        if ret.is_some() {
            commands.entity(entity).remove::<Task<bool>>();
        }
    }
}

@hanabi1224
Copy link
Author

@mockersf After discussion, I found it's better to just totally remove block_on and poll_once from user system code, and here's my proposed API change: #2691

@alice-i-cecile alice-i-cecile added S-Needs-Review C-Examples An addition or correction to our examples C-Performance A change motivated by improving speed, memory usage or compile times labels Sep 22, 2021
@Nilirad
Copy link
Contributor

Nilirad commented May 3, 2022

This PR adds a new example. Adding module and item level doc comments, as described in:

would be really useful to those who will browse examples.


I think it's sufficient to move the description on the top, as a module level doc comment.

I also think this should not be advertized as a benchmark, as examples are built without optimizations by default.

@alice-i-cecile alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Oct 10, 2022
@alice-i-cecile
Copy link
Member

Adding the Adopt-Me tag; I think this is valuable but the issues never got fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Tasks Tools for parallel and async work C-Examples An addition or correction to our examples C-Performance A change motivated by improving speed, memory usage or compile times S-Adopt-Me The original PR author has no intent to complete this work. Pick me up!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants