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
remove frequent global is_shutdown flag check on io operations #5300
Conversation
The global flag is still remaining, and used to prevent duplicated shutdown and new io operations after shutdown. On shutdown, the driver flip the shutdown flag of every pending io operations and wake them to fail with shutdown error. Fixes: tokio-rs#5227
Make timer driver's is_shutdown flag nonatomic and move it into the driver lock. On shtudown, the driver wake the all pending timer with shutdown error. Simillar to the tokio-rs#5300, but for timer driver not io driver. Refs: tokio-rs#5227, tokio-rs#5300
Make timer driver's is_shutdown flag nonatomic and move it into the driver lock. On shtudown, the driver wake the all pending timer with shutdown error. Simillar to the tokio-rs#5300, but for timer driver not io driver. Refs: tokio-rs#5227, tokio-rs#5300
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.
Looks reasonable to me.
tokio/src/runtime/io/registration.rs
Outdated
let fut = self.shared.readiness(interest); | ||
pin!(fut); | ||
|
||
crate::future::poll_fn(|cx| { | ||
if self.handle().is_shutdown() { | ||
return Poll::Ready(Err(io::Error::new( | ||
io::ErrorKind::Other, | ||
crate::util::error::RUNTIME_SHUTTING_DOWN_ERROR | ||
))); | ||
} | ||
let ev = crate::future::poll_fn(|cx| { | ||
Pin::new(&mut fut).poll(cx) | ||
}).await; |
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.
let ev = self.shared.readiness(interest).await;
Awesome! |
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.
Awesome job. Thanks
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.23.0` -> `1.24.1` | | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.23.0` -> `1.24.1` | --- ### Release Notes <details> <summary>tokio-rs/tokio</summary> ### [`v1.24.1`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.24.1): Tokio v1.24.1 [Compare Source](tokio-rs/tokio@tokio-1.24.0...tokio-1.24.1) This release fixes a compilation failure on targets without `AtomicU64` when using rustc older than 1.63. ([#​5356]) [#​5356]: tokio-rs/tokio#5356 ### [`v1.24.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.24.0): Tokio v1.24.0 [Compare Source](tokio-rs/tokio@tokio-1.23.1...tokio-1.24.0) The highlight of this release is the reduction of lock contention for all I/O operations ([#​5300](tokio-rs/tokio#5300)). We have received reports of up to a 20% improvement in CPU utilization and increased throughput for real-world I/O heavy applications. ##### Fixed - rt: improve native `AtomicU64` support detection ([#​5284]) ##### Added - rt: add configuration option for max number of I/O events polled from the OS per tick ([#​5186]) - rt: add an environment variable for configuring the default number of worker threads per runtime instance ([#​4250]) ##### Changed - sync: reduce MPSC channel stack usage ([#​5294]) - io: reduce lock contention in I/O operations ([#​5300]) - fs: speed up `read_dir()` by chunking operations ([#​5309]) - rt: use internal `ThreadId` implementation ([#​5329]) - test: don't auto-advance time when a `spawn_blocking` task is running ([#​5115]) [#​5186]: tokio-rs/tokio#5186 [#​5294]: tokio-rs/tokio#5294 [#​5284]: tokio-rs/tokio#5284 [#​4250]: tokio-rs/tokio#4250 [#​5300]: tokio-rs/tokio#5300 [#​5329]: tokio-rs/tokio#5329 [#​5115]: tokio-rs/tokio#5115 [#​5309]: tokio-rs/tokio#5309 ### [`v1.23.1`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.23.1): Tokio v1.23.1 [Compare Source](tokio-rs/tokio@tokio-1.23.0...tokio-1.23.1) This release forward ports changes from 1.18.4. ##### Fixed - net: fix Windows named pipe server builder to maintain option when toggling pipe mode ([#​5336]). [#​5336]: tokio-rs/tokio#5336 </details> --- ### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC44NC4xIiwidXBkYXRlZEluVmVyIjoiMzQuODQuMSJ9--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1703 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
The global flag is still remaining, and used to prevent duplicated shutdown and new io operations after shutdown.
On shutdown, the driver flip the shutdown flag of every pending io operations and wake them to fail with shutdown error.
Fixes: #5227
Motivation
I was load testing my tokio based RTMP ingestion server by attaching ffmpeg publishers each emits 4Mbps TCP traffic on AWS c5.4xlarge 16vcpu instance. With 100 publishers its CPU usage is about 7%. With 1k publishers(4Gbps) it's full 100%. Below is a flamegraph I've take from the scenario.
Here's a flamegraph from same 1k scenario after applying this PR. Its CPU usage is about 80%.
Solution
It's an implementation of @carllerche's suggestion from the issue. The global flag still exist but it's only checked on allocating IO handle, in which case you need to lock the IO driver anyway.