Directly schedule tasks instead of using Runnable::schedule #149
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
async_task::Runnable::schedule
has an "extra" Waker clone and drop whenever theschedule
function for the task captures variables to avoid deallocation during scheduling. We can avoid this by just... directly scheduling theRunnable
with the references that already exist. This PR just splits out the schedule functions into their own static functions and invokes that directly than go throughRunnable::schedule
. Benchmarking results:This seems to have a strong impact on the executor types that already have low spawning overhead (i.e. the StaticExecutors). Rerunning the benchmarks does seem to show some noise when testing for improvements, but overall generally seems to bias towards directly scheduling with at least a 5-10% perf gain on most of these benchmarks.
Commentary: I really dislike the fact that this is necessary to optimize the executors here.
Runnable::schedule
otherwise doesn't really have a reason to exist as an API if it's just going to be less efficient than directly scheduling the task. I suspect this is a combination of a lack of inlining due to dynamic dispatch (not sure why Rust does not devirtualize this call) and the lack of overhead form cloning the waker and dropping it to keep the task alive during scheduling. Not sure if it's advisable to update guidance forasync-task
based on this.