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

Darksonn · 2022-09-28T08:12:03Z

This PR removes OnceCell in favor of a const Mutex::new call when it is available (on sufficiently new rustc or with parking_lot).

Darksonn · 2022-09-28T08:37:51Z

I dropped the complicated cfg condition and put an allow(dead_code) on it because the conditions for when exactly we use it are too complicated. For example, the correct signal condition looks like this:

macro_rules! cfg_signal_internal {
    ($($item:item)*) => {
        $(
            #[cfg(any(feature = "signal", all(unix, feature = "process")))]
            #[cfg(not(loom))]
            $item
        )*
    }
}

Somehow I accidentally handled this correctly on the backports.

hawkw

looks good to me! some minor suggestions, but they're not important.

hawkw · 2022-09-28T17:38:18Z

tokio/src/macros/cfg.rs

+            #[cfg(not(all(
+                not(all(loom, test)),
+                any(
+                    feature = "parking_lot",
+                    not(tokio_no_const_mutex_new)
+                )
+            )))]


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?

I believe this is the simplest option.

hawkw · 2022-09-28T17:41:10Z

tokio/src/runtime/task/mod.rs

+        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)
+            }


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.`

tokio/src/util/mod.rs

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.21.2` -> `1.22.0` | | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.21.2` -> `1.22.0` | --- ### Release Notes <details> <summary>tokio-rs/tokio</summary> ### [`v1.22.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.22.0): Tokio v1.22.0 [Compare Source](tokio-rs/tokio@tokio-1.21.2...tokio-1.22.0) ##### Added - runtime: add `Handle::runtime_flavor` ([#5138]) - sync: add `Mutex::blocking_lock_owned` ([#5130]) - sync: add `Semaphore::MAX_PERMITS` ([#5144]) - sync: add `merge()` to semaphore permits ([#4948]) - sync: add `mpsc::WeakUnboundedSender` ([#5189]) ##### Added (unstable) - process: add `Command::process_group` ([#5114]) - runtime: export metrics about the blocking thread pool ([#5161]) - task: add `task::id()` and `task::try_id()` ([#5171]) ##### Fixed - macros: don't take ownership of futures in macros ([#5087]) - runtime: fix Stacked Borrows violation in `LocalOwnedTasks` ([#5099]) - runtime: mitigate ABA with 32-bit queue indices when possible ([#5042]) - task: wake local tasks to the local queue when woken by the same thread ([#5095]) - time: panic in release mode when `mark_pending` called illegally ([#5093]) - runtime: fix typo in expect message ([#5169]) - runtime: fix `unsync_load` on atomic types ([#5175]) - task: elaborate safety comments in task deallocation ([#5172]) - runtime: fix `LocalSet` drop in thread local ([#5179]) - net: remove libc type leakage in a public API ([#5191]) - runtime: update the alignment of `CachePadded` ([#5106]) ##### Changed - io: make `tokio::io::copy` continue filling the buffer when writer stalls ([#5066]) - runtime: remove `coop::budget` from `LocalSet::run_until` ([#5155]) - sync: make `Notify` panic safe ([#5154]) ##### Documented - io: fix doc for `write_i8` to use signed integers ([#5040]) - net: fix doc typos for TCP and UDP `set_tos` methods ([#5073]) - net: fix function name in `UdpSocket::recv` documentation ([#5150]) - sync: typo in `TryLockError` for `RwLock::try_write` ([#5160]) - task: document that spawned tasks execute immediately ([#5117]) - time: document return type of `timeout` ([#5118]) - time: document that `timeout` checks only before poll ([#5126]) - sync: specify return type of `oneshot::Receiver` in docs ([#5198]) ##### Internal changes - runtime: use const `Mutex::new` for globals ([#5061]) - runtime: remove `Option` around `mio::Events` in io driver ([#5078]) - runtime: remove a conditional compilation clause ([#5104]) - runtime: remove a reference to internal time handle ([#5107]) - runtime: misc time driver cleanup ([#5120]) - runtime: move signal driver to runtime module ([#5121]) - runtime: signal driver now uses I/O driver directly ([#5125]) - runtime: start decoupling I/O driver and I/O handle ([#5127]) - runtime: switch `io::handle` refs with scheduler:Handle ([#5128]) - runtime: remove Arc from I/O driver ([#5134]) - runtime: use signal driver handle via `scheduler::Handle` ([#5135]) - runtime: move internal clock fns out of context ([#5139]) - runtime: remove `runtime::context` module ([#5140]) - runtime: keep driver cfgs in `driver.rs` ([#5141]) - runtime: add `runtime::context` to unify thread-locals ([#5143]) - runtime: rename some confusing internal variables/fns ([#5151]) - runtime: move `coop` mod into `runtime` ([#5152]) - runtime: move budget state to context thread-local ([#5157]) - runtime: move park logic into runtime module ([#5158]) - runtime: move `Runtime` into its own file ([#5159]) - runtime: unify entering a runtime with `Handle::enter` ([#5163]) - runtime: remove handle reference from each scheduler ([#5166]) - runtime: move `enter` into `context` ([#5167]) - runtime: combine context and entered thread-locals ([#5168]) - runtime: fix accidental unsetting of current handle ([#5178]) - runtime: move `CoreStage` methods to `Core` ([#5182]) - sync: name mpsc semaphore types ([#5146]) [#4948]: tokio-rs/tokio#4948 [#5040]: tokio-rs/tokio#5040 [#5042]: tokio-rs/tokio#5042 [#5061]: tokio-rs/tokio#5061 [#5066]: tokio-rs/tokio#5066 [#5073]: tokio-rs/tokio#5073 [#5078]: tokio-rs/tokio#5078 [#5087]: tokio-rs/tokio#5087 [#5093]: tokio-rs/tokio#5093 [#5095]: tokio-rs/tokio#5095 [#5099]: tokio-rs/tokio#5099 [#5104]: tokio-rs/tokio#5104 [#5106]: tokio-rs/tokio#5106 [#5107]: tokio-rs/tokio#5107 [#5114]: tokio-rs/tokio#5114 [#5117]: tokio-rs/tokio#5117 [#5118]: tokio-rs/tokio#5118 [#5120]: tokio-rs/tokio#5120 [#5121]: tokio-rs/tokio#5121 [#5125]: tokio-rs/tokio#5125 [#5126]: tokio-rs/tokio#5126 [#5127]: tokio-rs/tokio#5127 [#5128]: tokio-rs/tokio#5128 [#5130]: tokio-rs/tokio#5130 [#5134]: tokio-rs/tokio#5134 [#5135]: tokio-rs/tokio#5135 [#5138]: tokio-rs/tokio#5138 [#5138]: tokio-rs/tokio#5138 [#5139]: tokio-rs/tokio#5139 [#5140]: tokio-rs/tokio#5140 [#5141]: tokio-rs/tokio#5141 [#5143]: tokio-rs/tokio#5143 [#5144]: tokio-rs/tokio#5144 [#5144]: tokio-rs/tokio#5144 [#5146]: tokio-rs/tokio#5146 [#5150]: tokio-rs/tokio#5150 [#5151]: tokio-rs/tokio#5151 [#5152]: tokio-rs/tokio#5152 [#5154]: tokio-rs/tokio#5154 [#5155]: tokio-rs/tokio#5155 [#5157]: tokio-rs/tokio#5157 [#5158]: tokio-rs/tokio#5158 [#5159]: tokio-rs/tokio#5159 [#5160]: tokio-rs/tokio#5160 [#5161]: tokio-rs/tokio#5161 [#5163]: tokio-rs/tokio#5163 [#5166]: tokio-rs/tokio#5166 [#5167]: tokio-rs/tokio#5167 [#5168]: tokio-rs/tokio#5168 [#5169]: tokio-rs/tokio#5169 [#5171]: tokio-rs/tokio#5171 [#5172]: tokio-rs/tokio#5172 [#5175]: tokio-rs/tokio#5175 [#5178]: tokio-rs/tokio#5178 [#5179]: tokio-rs/tokio#5179 [#5182]: tokio-rs/tokio#5182 [#5189]: tokio-rs/tokio#5189 [#5191]: tokio-rs/tokio#5191 [#5198]: tokio-rs/tokio#5198 </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. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1651 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>

tokio: use const Mutex::new for globals

34839f2

Darksonn added the A-tokio Area: The main tokio crate label Sep 28, 2022

github-actions bot added the R-loom Run loom tests on this PR label Sep 28, 2022

Simplify cfgs for OnceCell

b5310af

Noah-Kennedy approved these changes Sep 28, 2022

View reviewed changes

hawkw approved these changes Sep 28, 2022

View reviewed changes

Darksonn merged commit f4e08ae into master Sep 28, 2022

Darksonn deleted the alice/const_mutex_new branch September 28, 2022 19:21

dbischof90 pushed a commit to dbischof90/tokio that referenced this pull request Oct 1, 2022

tokio: use const Mutex::new for globals (tokio-rs#5061)

87b2d9e

Darksonn mentioned this pull request Nov 17, 2022

chore: prepare Tokio v1.22.0 #5203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Darksonn commented Sep 28, 2022

Darksonn commented Sep 28, 2022

hawkw left a comment

hawkw Sep 28, 2022

Darksonn Sep 28, 2022

hawkw Sep 28, 2022

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

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

Conversation

Darksonn commented Sep 28, 2022

Darksonn commented Sep 28, 2022

hawkw left a comment

Choose a reason for hiding this comment

hawkw Sep 28, 2022

Choose a reason for hiding this comment

Darksonn Sep 28, 2022

Choose a reason for hiding this comment

hawkw Sep 28, 2022

Choose a reason for hiding this comment

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

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