-
-
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
remove frequent global is_shutdown flag check on io operations #5300
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
HyeonuPark
added a commit
to HyeonuPark/tokio
that referenced
this pull request
Dec 17, 2022
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
HyeonuPark
added a commit
to HyeonuPark/tokio
that referenced
this pull request
Dec 17, 2022
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
Darksonn
added
A-tokio
Area: The main tokio crate
M-time
Module: tokio/time
T-performance
Topic: performance and benchmarks
labels
Dec 17, 2022
Darksonn
approved these changes
Dec 17, 2022
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
Comment on lines
232
to
237
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;
Noah-Kennedy
approved these changes
Dec 17, 2022
Awesome! |
carllerche
approved these changes
Dec 27, 2022
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
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Jan 8, 2023
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 <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1703 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>
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
M-time
Module: tokio/time
R-loom
Run loom tests on this PR
T-performance
Topic: performance and benchmarks
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.
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.