-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
configable budget for runtime #5775
Conversation
@Darksonn @Noah-Kennedy @carllerche , I can't figure out why the minrust job is failing |
Hi, I would like to breakdown the work into below plan
|
Summary of benchmarkAfter running a similar duration, we can see with budget=32, it has better latency performance according to 99.9% quantile, stdev and mean value high priority with budget = 32
high priority with budget = 128
machine
|
pub(crate) fn budget<R>(f: impl FnOnce() -> R) -> R { | ||
with_budget(Budget::initial(), f) | ||
with_budget( | ||
context::get_init_budget().unwrap_or_else(|_| Budget::initial()), | ||
f, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces an extra thread-local access on every poll. @carllerche Thoughts? I think you said that you were working on reducing them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be removing TLS calls in the hot path at this point. It should be possible to build this without any more TLS lookups.
I'm in the middle of a big scheduler refactor. It will be easier to make this change after the refactor lands. Ideally in 2-3 weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For stable api, get_init_budget()
return value directly instead of accessing the TLS, see below
pub(super) fn get_init_budget() -> Result<coop::Budget, AccessError> {
#[cfg(not(tokio_unstable))]
{
Ok(coop::Budget::default())
}
#[cfg(tokio_unstable)]
{
CONTEXT.try_with(|ctx| ctx.initial_budget.get())
}
}
I'm going to close this, because we don't want to add additional uses of the thread-local at this time. |
@Darksonn , may I know what is plan forward, would we consider to work on this after some factors are done? |
If you want to start working on this again with a different approach, then I'm open to that. You no longer need to wait for Carl's work to finish. |
addressing #5696
First experiment with configurable coop budget
Also create an example project for benchmarking