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

tokio: use const Mutex::new for globals #5061

Merged
merged 2 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
31 changes: 31 additions & 0 deletions tokio/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,17 @@ const ADDR_OF_PROBE: &str = r#"
}
"#;

const CONST_MUTEX_NEW_PROBE: &str = r#"
{
static MY_MUTEX: ::std::sync::Mutex<i32> = ::std::sync::Mutex::new(1);
*MY_MUTEX.lock().unwrap()
}
"#;

fn main() {
let mut enable_const_thread_local = false;
let mut enable_addr_of = false;
let mut enable_const_mutex_new = false;

match AutoCfg::new() {
Ok(ac) => {
Expand Down Expand Up @@ -57,6 +65,21 @@ fn main() {
enable_addr_of = true;
}
}

// The `Mutex::new` method was made const in 1.63.
if ac.probe_rustc_version(1, 64) {
enable_const_mutex_new = true;
} else if ac.probe_rustc_version(1, 63) {
// This compiler claims to be 1.63, but there are some nightly
// compilers that claim to be 1.63 without supporting the
// feature. Explicitly probe to check if code using them
// compiles.
//
// The oldest nightly that supports the feature is 2022-06-20.
if ac.probe_expression(CONST_MUTEX_NEW_PROBE) {
enable_const_mutex_new = true;
}
}
}

Err(e) => {
Expand Down Expand Up @@ -86,6 +109,14 @@ fn main() {
autocfg::emit("tokio_no_addr_of")
}

if !enable_const_mutex_new {
// To disable this feature on compilers that support it, you can
// explicitly pass this flag with the following environment variable:
//
// RUSTFLAGS="--cfg tokio_no_const_mutex_new"
autocfg::emit("tokio_no_const_mutex_new")
}

let target = ::std::env::var("TARGET").unwrap_or_default();

// We emit cfgs instead of using `target_family = "wasm"` that requires Rust 1.54.
Expand Down
6 changes: 6 additions & 0 deletions tokio/src/loom/std/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ impl<T> Mutex<T> {
Mutex(sync::Mutex::new(t))
}

#[inline]
#[cfg(not(tokio_no_const_mutex_new))]
pub(crate) const fn const_new(t: T) -> Mutex<T> {
Mutex(sync::Mutex::new(t))
}

#[inline]
pub(crate) fn lock(&self) -> MutexGuard<'_, T> {
match self.0.lock() {
Expand Down
30 changes: 30 additions & 0 deletions tokio/src/macros/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,36 @@ macro_rules! cfg_not_has_atomic_u64 {
}
}

macro_rules! cfg_has_const_mutex_new {
($($item:item)*) => {
$(
#[cfg(all(
not(all(loom, test)),
any(
feature = "parking_lot",
not(tokio_no_const_mutex_new)
)
))]
$item
)*
}
}

macro_rules! cfg_not_has_const_mutex_new {
($($item:item)*) => {
$(
#[cfg(not(all(
not(all(loom, test)),
any(
feature = "parking_lot",
not(tokio_no_const_mutex_new)
)
)))]
Comment on lines +502 to +508
Copy link
Member

Choose a reason for hiding this comment

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

nit, take it or leave it: would it make sense to simplify this by just having the build script not set the tokio_no_const_mutex_new cfg when the loom and test cfgs are set? or is this simpler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this is the simplest option.

$item
)*
}
}

macro_rules! cfg_not_wasi {
($($item:item)*) => {
$(
Expand Down
19 changes: 15 additions & 4 deletions tokio/src/process/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use crate::process::kill::Kill;
use crate::process::SpawnedChild;
use crate::signal::unix::driver::Handle as SignalHandle;
use crate::signal::unix::{signal, Signal, SignalKind};
use crate::util::once_cell::OnceCell;

use mio::event::Source;
use mio::unix::SourceFd;
Expand Down Expand Up @@ -64,10 +63,22 @@ impl Kill for StdChild {
}
}

fn get_orphan_queue() -> &'static OrphanQueueImpl<StdChild> {
static ORPHAN_QUEUE: OnceCell<OrphanQueueImpl<StdChild>> = OnceCell::new();
cfg_not_has_const_mutex_new! {
fn get_orphan_queue() -> &'static OrphanQueueImpl<StdChild> {
use crate::util::once_cell::OnceCell;

ORPHAN_QUEUE.get(OrphanQueueImpl::new)
static ORPHAN_QUEUE: OnceCell<OrphanQueueImpl<StdChild>> = OnceCell::new();

ORPHAN_QUEUE.get(OrphanQueueImpl::new)
}
}

cfg_has_const_mutex_new! {
fn get_orphan_queue() -> &'static OrphanQueueImpl<StdChild> {
static ORPHAN_QUEUE: OrphanQueueImpl<StdChild> = OrphanQueueImpl::new();

&ORPHAN_QUEUE
}
}

pub(crate) struct GlobalOrphanQueue;
Expand Down
19 changes: 15 additions & 4 deletions tokio/src/process/unix/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,21 @@ pub(crate) struct OrphanQueueImpl<T> {
}

impl<T> OrphanQueueImpl<T> {
pub(crate) fn new() -> Self {
Self {
sigchild: Mutex::new(None),
queue: Mutex::new(Vec::new()),
cfg_not_has_const_mutex_new! {
pub(crate) fn new() -> Self {
Self {
sigchild: Mutex::new(None),
queue: Mutex::new(Vec::new()),
}
}
}

cfg_has_const_mutex_new! {
pub(crate) const fn new() -> Self {
Self {
sigchild: Mutex::const_new(None),
queue: Mutex::const_new(Vec::new()),
}
}
}

Expand Down
38 changes: 26 additions & 12 deletions tokio/src/runtime/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,21 +507,35 @@ impl Id {
}

cfg_not_has_atomic_u64! {
pub(crate) fn next() -> Self {
use crate::util::once_cell::OnceCell;
use crate::loom::sync::Mutex;

fn init_next_id() -> Mutex<u64> {
Mutex::new(1)
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)
}
}

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

let next_id = NEXT_ID.get(init_next_id);
let mut lock = next_id.lock();
let id = *lock;
*lock += 1;
Self(id)
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)
}
Comment on lines +510 to +538
Copy link
Member

Choose a reason for hiding this comment

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

hmm, it might be nice if the logic that actually advances the next id was not duplicated across cfgs, but i'm not sure if there's a nice way to do that without changes to the macro to take a block instead of an item, which would make it different from the rest of the cfg_foo macros...probably not worth bothering with.`

}
}

Expand Down
10 changes: 2 additions & 8 deletions tokio/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,8 @@ cfg_io_driver! {
#[cfg(feature = "rt")]
pub(crate) mod atomic_cell;

cfg_has_atomic_u64! {
#[cfg(any(feature = "signal", all(unix, feature = "process")))]
pub(crate) mod once_cell;
}
cfg_not_has_atomic_u64! {
#[cfg(any(feature = "rt", feature = "signal", all(unix, feature = "process")))]
pub(crate) mod once_cell;
}
#[cfg(any(feature = "rt", feature = "signal", feature = "process"))]
pub(crate) mod once_cell;
hawkw marked this conversation as resolved.
Show resolved Hide resolved

#[cfg(any(
// io driver uses `WakeList` directly
Expand Down
2 changes: 1 addition & 1 deletion tokio/src/util/once_cell.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg_attr(loom, allow(dead_code))]
#![allow(dead_code)]
use std::cell::UnsafeCell;
use std::mem::MaybeUninit;
use std::sync::Once;
Expand Down