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

rt: refactor current-thread scheduler #4377

Merged
merged 12 commits into from
Jan 7, 2022
Merged

rt: refactor current-thread scheduler #4377

merged 12 commits into from
Jan 7, 2022

Conversation

carllerche
Copy link
Member

This PR does some refactoring to the current-thread scheduler bringing it closer to the structure of the multi-threaded scheduler. More specifically, the core scheduler data is stored in a Core struct and that struct is passed around as a "token" indicating permission to do work. The Core structure is also stored in the thread-local context.

This refactor is intended to support #4373, making it easier to track counters in more locations in the current-thread scheduler.

I tried to keep commits small, but the "set Core in thread-local context" is both the biggest commit and the key one.

@github-actions github-actions bot added the R-loom Run loom tests on this PR label Jan 4, 2022
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime labels Jan 5, 2022
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

LGTM just a few questions

tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Show resolved Hide resolved
tokio/src/runtime/basic_scheduler.rs Show resolved Hide resolved
Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

LGTM

tokio/src/runtime/basic_scheduler.rs Outdated Show resolved Hide resolved
Co-authored-by: Lucio Franco <luciofranco14@gmail.com>
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me!

@carllerche carllerche merged commit cc8ad36 into master Jan 7, 2022
@carllerche carllerche deleted the refactor-basic-rt branch January 7, 2022 01:19
taiki-e added a commit that referenced this pull request Jan 11, 2022
This reverts commit cc8ad36.

That commit caused double panic in hyper's test suite.
@taiki-e taiki-e mentioned this pull request Jan 11, 2022
carllerche added a commit that referenced this pull request Jan 11, 2022
carllerche added a commit that referenced this pull request Jan 11, 2022
carllerche added a commit that referenced this pull request Jan 11, 2022
carllerche added a commit that referenced this pull request Jan 12, 2022
Re-applies #4377 and fixes the bug resulting in Hyper's double panic.

Revert: #4394

Original PR:

This PR does some refactoring to the current-thread scheduler bringing it closer to the structure of the
multi-threaded scheduler. More specifically, the core scheduler data is stored in a Core struct and that
struct is passed around as a "token" indicating permission to do work. The Core structure is also stored
in the thread-local context.

This refactor is intended to support #4373, making it easier to track counters in more locations in the
current-thread scheduler.

I tried to keep commits small, but the "set Core in thread-local context" is both the biggest commit and
the key one.
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-runtime Module: tokio/runtime R-loom Run loom tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants