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

remove frequent global is_shutdown flag check on io operations #5300

Merged
merged 2 commits into from Dec 27, 2022

Conversation

HyeonuPark
Copy link
Contributor

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.

flamegraph before

Here's a flamegraph from same 1k scenario after applying this PR. Its CPU usage is about 80%.

flamegraph after

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.

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
@github-actions github-actions bot added the R-loom Run loom tests on this PR label Dec 17, 2022
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 Darksonn added A-tokio Area: The main tokio crate M-time Module: tokio/time T-performance Topic: performance and benchmarks labels Dec 17, 2022
Copy link
Contributor

@Darksonn Darksonn left a 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.

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;
Copy link
Contributor

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
Copy link
Contributor

Awesome!

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job. Thanks

@carllerche carllerche merged commit 9af2f5e into tokio-rs:master Dec 27, 2022
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. ([#&#8203;5356])

[#&#8203;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 ([#&#8203;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 ([#&#8203;5284])

##### Added

-   rt: add configuration option for max number of I/O events polled from the OS
    per tick ([#&#8203;5186])
-   rt: add an environment variable for configuring the default number of worker
    threads per runtime instance ([#&#8203;4250])

##### Changed

-   sync: reduce MPSC channel stack usage ([#&#8203;5294])
-   io: reduce lock contention in I/O operations  ([#&#8203;5300])
-   fs: speed up `read_dir()` by chunking operations ([#&#8203;5309])
-   rt: use internal `ThreadId` implementation ([#&#8203;5329])
-   test: don't auto-advance time when a `spawn_blocking` task is running ([#&#8203;5115])

[#&#8203;5186]: tokio-rs/tokio#5186

[#&#8203;5294]: tokio-rs/tokio#5294

[#&#8203;5284]: tokio-rs/tokio#5284

[#&#8203;4250]: tokio-rs/tokio#4250

[#&#8203;5300]: tokio-rs/tokio#5300

[#&#8203;5329]: tokio-rs/tokio#5329

[#&#8203;5115]: tokio-rs/tokio#5115

[#&#8203;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 ([#&#8203;5336]).

[#&#8203;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>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

perf - io::Handle::is_shutdown acquires a global lock on every IO op
4 participants