Skip to content

Commit

Permalink
Revisions from reviewing
Browse files Browse the repository at this point in the history
* Update tick and interval types from `u8` to `u32`.
* Expand documentation for the interval knobs to explain why different settings are useful.
  • Loading branch information
aturon committed May 17, 2022
1 parent 968371e commit 6e9c850
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 23 deletions.
10 changes: 5 additions & 5 deletions tokio/src/runtime/basic_scheduler.rs
Expand Up @@ -48,7 +48,7 @@ struct Core {
spawner: Spawner,

/// Current tick
tick: u8,
tick: u32,

/// Runtime driver
///
Expand All @@ -59,10 +59,10 @@ struct Core {
metrics: MetricsBatch,

/// How many ticks before pulling a task from the global/remote queue?
global_queue_interval: u8,
global_queue_interval: u32,

/// How many ticks before yielding to the driver for timer and I/O events?
event_interval: u8,
event_interval: u32,
}

#[derive(Clone)]
Expand Down Expand Up @@ -122,8 +122,8 @@ impl BasicScheduler {
handle_inner: HandleInner,
before_park: Option<Callback>,
after_unpark: Option<Callback>,
global_queue_interval: u8,
event_interval: u8,
global_queue_interval: u32,
event_interval: u32,
) -> BasicScheduler {
let unpark = driver.unpark();

Expand Down
33 changes: 22 additions & 11 deletions tokio/src/runtime/builder.rs
Expand Up @@ -80,10 +80,10 @@ pub struct Builder {
pub(super) keep_alive: Option<Duration>,

/// How many ticks before pulling a task from the global/remote queue?
pub(super) global_queue_interval: u8,
pub(super) global_queue_interval: u32,

/// How many ticks before yielding to the driver for timer and I/O events?
pub(super) event_interval: u8,
pub(super) event_interval: u32,
}

pub(crate) type ThreadNameFn = std::sync::Arc<dyn Fn() -> String + Send + Sync + 'static>;
Expand All @@ -105,13 +105,13 @@ impl Builder {
/// [`LocalSet`]: crate::task::LocalSet
pub fn new_current_thread() -> Builder {
/// How often to check the remote queue first.
const REMOTE_FIRST_INTERVAL: u8 = 31;
const REMOTE_FIRST_INTERVAL: u32 = 31;

/// Max number of tasks to poll per tick.
#[cfg(loom)]
const MAX_TASKS_PER_TICK: u8 = 4;
const MAX_TASKS_PER_TICK: u32 = 4;
#[cfg(not(loom))]
const MAX_TASKS_PER_TICK: u8 = 61;
const MAX_TASKS_PER_TICK: u32 = 61;

Builder::new(
Kind::CurrentThread,
Expand All @@ -132,7 +132,7 @@ impl Builder {
/// The same value is used to control when to yield to the driver for events.
///
/// The number is fairly arbitrary. I believe this value was copied from golang.
const GLOBAL_POLL_INTERVAL: u8 = 61;
const GLOBAL_POLL_INTERVAL: u32 = 61;

Builder::new(
Kind::MultiThread,
Expand All @@ -145,7 +145,7 @@ impl Builder {
/// values.
///
/// Configuration methods can be chained on the return value.
pub(crate) fn new(kind: Kind, global_queue_interval: u8, event_interval: u8) -> Builder {
pub(crate) fn new(kind: Kind, global_queue_interval: u32, event_interval: u32) -> Builder {
Builder {
kind,

Expand Down Expand Up @@ -595,8 +595,12 @@ impl Builder {
///
/// A scheduler "tick" roughly corresponds to one `poll` invocation on a task. Schedulers
/// have a local queue of already-claimed tasks, and a global queue of incoming tasks.
///
/// Setting the interval to a smaller value increases the fairness of the scheduler,
/// at the cost of more synchronization overhead.
/// at the cost of more synchronization overhead. That can be beneficial for prioritizing
/// getting started on new work, especially if tasks frequently yield rather than complete
/// or await on further I/O. Conversely, a higher value prioritizes existing work, and
/// is a good choice when most tasks quickly complete polling.
///
/// # Examples
///
Expand All @@ -609,7 +613,7 @@ impl Builder {
/// .build();
/// # }
/// ```
pub fn global_queue_interval(&mut self, val: u8) -> &mut Self {
pub fn global_queue_interval(&mut self, val: u32) -> &mut Self {
self.global_queue_interval = val;
self
}
Expand All @@ -618,9 +622,16 @@ impl Builder {
/// external events (timers, I/O, and so on).
///
/// A scheduler "tick" roughly corresponds to one `poll` invocation on a task.
///
/// Setting the event interval determines the effective "priority" of delivering
/// these external events (which may wake up additional tasks), compared to
/// executing tasks that are currently ready to run.
/// executing tasks that are currently ready to run. A smaller value is useful
/// when tasks frequently spend a long time in polling, or frequently yield,
/// which can result in overly long delays picking up I/O events. Conversely,
/// picking up new events requires extra synchronization and syscall overhead,
/// so if tasks generally complete their polling quickly, a higher event interval
/// will minimize that overhead while still keeping the scheduler responsive to
/// events.
///
/// # Examples
///
Expand All @@ -633,7 +644,7 @@ impl Builder {
/// .build();
/// # }
/// ```
pub fn event_interval(&mut self, val: u8) -> &mut Self {
pub fn event_interval(&mut self, val: u32) -> &mut Self {
self.event_interval = val;
self
}
Expand Down
4 changes: 2 additions & 2 deletions tokio/src/runtime/thread_pool/mod.rs
Expand Up @@ -51,8 +51,8 @@ impl ThreadPool {
handle_inner: HandleInner,
before_park: Option<Callback>,
after_unpark: Option<Callback>,
global_queue_interval: u8,
event_interval: u8,
global_queue_interval: u32,
event_interval: u32,
) -> (ThreadPool, Launch) {
let parker = Parker::new(driver);
let (shared, launch) = worker::create(
Expand Down
10 changes: 5 additions & 5 deletions tokio/src/runtime/thread_pool/worker.rs
Expand Up @@ -87,7 +87,7 @@ pub(super) struct Worker {
/// Core data
struct Core {
/// Used to schedule bookkeeping tasks every so often.
tick: u8,
tick: u32,

/// When a task is scheduled from a worker, it is stored in this slot. The
/// worker will check this slot for a task **before** checking the run
Expand Down Expand Up @@ -119,10 +119,10 @@ struct Core {
rand: FastRand,

/// How many ticks before pulling a task from the global/remote queue?
global_queue_interval: u8,
global_queue_interval: u32,

/// How many ticks before yielding to the driver for timer and I/O events?
event_interval: u8,
event_interval: u32,
}

/// State shared across all workers
Expand Down Expand Up @@ -204,8 +204,8 @@ pub(super) fn create(
handle_inner: HandleInner,
before_park: Option<Callback>,
after_unpark: Option<Callback>,
global_queue_interval: u8,
event_interval: u8,
global_queue_interval: u32,
event_interval: u32,
) -> (Arc<Shared>, Launch) {
let mut cores = Vec::with_capacity(size);
let mut remotes = Vec::with_capacity(size);
Expand Down

0 comments on commit 6e9c850

Please sign in to comment.