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: implement task::Id using StaticAtomicU64 #5282

Merged
merged 3 commits into from
Dec 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions tokio/src/loom/std/atomic_u64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
// `#[cfg(target_has_atomic = "64")]`.
// Refs: https://github.com/rust-lang/rust/tree/master/src/librustc_target
cfg_has_atomic_u64! {
pub(crate) use std::sync::atomic::AtomicU64;
#[path = "atomic_u64_native.rs"]
mod imp;
}

cfg_not_has_atomic_u64! {
#[path = "atomic_u64_as_mutex.rs"]
mod atomic_u64_as_mutex;

pub(crate) use atomic_u64_as_mutex::AtomicU64;
mod imp;
}

pub(crate) use imp::{AtomicU64, StaticAtomicU64};
18 changes: 12 additions & 6 deletions tokio/src/loom/std/atomic_u64_as_mutex.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
use crate::loom::sync::Mutex;
use std::sync::atomic::Ordering;

cfg_has_const_mutex_new! {
#[path = "atomic_u64_static_const_new.rs"]
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm experimenting with a way to have less code within the cfg macros. This should help rustfmt apply to more code.

mod static_macro;
}

cfg_not_has_const_mutex_new! {
#[path = "atomic_u64_static_once_cell.rs"]
mod static_macro;
}

pub(crate) use static_macro::StaticAtomicU64;

#[derive(Debug)]
pub(crate) struct AtomicU64 {
inner: Mutex<u64>,
}

impl AtomicU64 {
pub(crate) fn new(val: u64) -> Self {
Self {
inner: Mutex::new(val),
}
}

pub(crate) fn load(&self, _: Ordering) -> u64 {
*self.inner.lock()
}
Expand Down
4 changes: 4 additions & 0 deletions tokio/src/loom/std/atomic_u64_native.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub(crate) use std::sync::atomic::{AtomicU64, Ordering};

/// Alias `AtomicU64` to `StaticAtomicU64`
pub(crate) type StaticAtomicU64 = AtomicU64;
12 changes: 12 additions & 0 deletions tokio/src/loom/std/atomic_u64_static_const_new.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use super::AtomicU64;
use crate::loom::sync::Mutex;

pub(crate) type StaticAtomicU64 = AtomicU64;

impl AtomicU64 {
pub(crate) const fn new(val: u64) -> Self {
Self {
inner: Mutex::const_new(val),
}
}
}
36 changes: 36 additions & 0 deletions tokio/src/loom/std/atomic_u64_static_once_cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use super::AtomicU64;
use crate::loom::sync::{Mutex, atomic::Ordering};
use crate::util::once_cell::OnceCell;

pub(crate) struct StaticAtomicU64 {
init: u64,
Copy link
Member Author

Choose a reason for hiding this comment

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

It is a bit annoying to store the initial value forever, but this is only for platforms that do not have AtomicU64 and no const fn Mutex::new() (i.e. old versions of Rust), so it isn't a big deal IMO.

cell: OnceCell<Mutex<u64>>,
}

impl AtomicU64 {
pub(crate) fn new(val: u64) -> Self {
Self {
inner: Mutex::new(val),
}
}
}

impl StaticAtomicU64 {
pub(crate) const fn new(val: u64) -> StaticAtomicU64 {
StaticAtomicU64 {
init: val,
cell: OnceCell::new(),
}
}

pub(crate) fn fetch_add(&self, val: u64, order: Ordering) -> u64 {
let mut lock = self.inner().lock();
let prev = *lock;
*lock = prev + val;
prev
}

fn inner(&self) -> &Mutex<u64> {
self.cell.get(|| Mutex::new(self.init))
}
}
2 changes: 1 addition & 1 deletion tokio/src/loom/std/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub(crate) mod sync {
pub(crate) mod atomic {
pub(crate) use crate::loom::std::atomic_u16::AtomicU16;
pub(crate) use crate::loom::std::atomic_u32::AtomicU32;
pub(crate) use crate::loom::std::atomic_u64::AtomicU64;
pub(crate) use crate::loom::std::atomic_u64::{AtomicU64, StaticAtomicU64};
pub(crate) use crate::loom::std::atomic_usize::AtomicUsize;

pub(crate) use std::sync::atomic::{fence, AtomicBool, AtomicPtr, AtomicU8, Ordering};
Expand Down
51 changes: 4 additions & 47 deletions tokio/src/runtime/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,55 +562,12 @@ impl fmt::Display for Id {
}

impl Id {
// When 64-bit atomics are available, use a static `AtomicU64` counter to
// generate task IDs.
//
// Note(eliza): we _could_ just use `crate::loom::AtomicU64`, which switches
// between an atomic and mutex-based implementation here, rather than having
// two separate functions for targets with and without 64-bit atomics.
// However, because we can't use the mutex-based implementation in a static
// initializer directly, the 32-bit impl also has to use a `OnceCell`, and I
// thought it was nicer to avoid the `OnceCell` overhead on 64-bit
// platforms...
cfg_has_atomic_u64! {
pub(crate) fn next() -> Self {
use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
static NEXT_ID: AtomicU64 = AtomicU64::new(1);
Self(NEXT_ID.fetch_add(1, Relaxed))
}
}

cfg_not_has_atomic_u64! {
cfg_has_const_mutex_new! {
pub(crate) fn next() -> Self {
use crate::loom::sync::Mutex;
static NEXT_ID: Mutex<u64> = Mutex::const_new(1);

let mut lock = NEXT_ID.lock();
let id = *lock;
*lock += 1;
Self(id)
}
}
pub(crate) fn next() -> Self {
use crate::loom::sync::atomic::{Ordering::Relaxed, StaticAtomicU64};

cfg_not_has_const_mutex_new! {
pub(crate) fn next() -> Self {
use crate::util::once_cell::OnceCell;
use crate::loom::sync::Mutex;
static NEXT_ID: StaticAtomicU64 = StaticAtomicU64::new(1);

fn init_next_id() -> Mutex<u64> {
Mutex::new(1)
}

static NEXT_ID: OnceCell<Mutex<u64>> = OnceCell::new();

let next_id = NEXT_ID.get(init_next_id);
let mut lock = next_id.lock();
let id = *lock;
*lock += 1;
Self(id)
}
}
Self(NEXT_ID.fetch_add(1, Relaxed))
}

pub(crate) fn as_u64(&self) -> u64 {
Expand Down
4 changes: 2 additions & 2 deletions tokio/src/util/once_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<T> OnceCell<T> {
/// If the `init` closure panics, then the `OnceCell` is poisoned and all
/// future calls to `get` will panic.
#[inline]
pub(crate) fn get(&self, init: fn() -> T) -> &T {
pub(crate) fn get(&self, init: impl FnOnce() -> T) -> &T {

Choose a reason for hiding this comment

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

typo on line 22: `intiailizing' -> initializing

if !self.once.is_completed() {
self.do_init(init);
}
Expand All @@ -41,7 +41,7 @@ impl<T> OnceCell<T> {
}

#[cold]
fn do_init(&self, init: fn() -> T) {
fn do_init(&self, init: impl FnOnce() -> T) {
let value_ptr = self.value.get() as *mut T;

self.once.call_once(|| {
Expand Down