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

yield_now not yielding since MAX_TASKS_PER_TICK in LocalSet is hardcoded #5209

Closed
arielb1 opened this issue Nov 18, 2022 · 3 comments
Closed
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-task Module: tokio/task

Comments

@arielb1
Copy link

arielb1 commented Nov 18, 2022

Version

Tokio 1.21.2

Platform

Linux 5.15.0-53-generic #59-Ubuntu SMP Mon Oct 17 18:53:30 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Description

The MAX_TICKS_PER_TASK parameter in LocalSet is hardcoded to be 61.

This means that if, while a task is waiting for IO, there is a task in a LocalSet that is performing

loop {
    expensive_computation();
    tokio::task::yield_now().await;
}

The expensive_computation will run up to 61 times before IO is polled, which will cause high tail latencies.

cc @carllerche

@arielb1 arielb1 added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Nov 18, 2022
@arielb1 arielb1 changed the title High tail latencies since MAX_TASKS_PER_TICK in LocalSet is hardcoded yield_now not yielding since MAX_TASKS_PER_TICK in LocalSet is hardcoded Nov 18, 2022
@arielb1
Copy link
Author

arielb1 commented Nov 18, 2022

I would also like some way to have a version of yield_now that makes the task not wake up until IO is polled.

Currently I do some evil hack that creates a pipe and waits for the pipe to be readable, but I hate that.

@Darksonn Darksonn added the M-task Module: tokio/task label Nov 18, 2022
@Darksonn
Copy link
Contributor

In FuturesUnordered, we make it always yield if the task wakes itself. That could be one approach to this.

carllerche added a commit that referenced this issue Nov 30, 2022
Previously, calling `task::yield_now().await` would yield the current
task to the scheduler, but the scheduler would poll it again before
polling the resource drivers. This behavior can result in starving the
resource drivers.

This patch creates a queue tracking yielded tasks. The scheduler
notifies those tasks **after** polling the resource drivers.

Refs: #5209
@carllerche
Copy link
Member

I believe #5223 should resolve your issue here. While it would still be nice for LocalSet to be configurable, yielding no longer triggers this.

@arielb1 arielb1 closed this as completed Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-task Module: tokio/task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants