Tokio: Const constructors for Semaphore, Mutex, etc. with parking_lot.

[steffahn]

I’m noticing that with the parking_lot feature enabled, the type tokio::sync::Semaphore is made up of types that can all be constructed with const fns. Putting a Mutex into a static variable is a common pattern to simulate global variables in Rust; without something like const fn new available, one also needs to wrap the variable into a lazy_static or something similar. Thus the following seems like a valuable addition to me.

I propose that a few functions are made conditionally const, when the parking_lot feature is enabled. I’m not sure yet how nice this can work out in the documentation and how bad it would look in the source code though. If making e.g. Mutex::new conditionally const causes too much churn, an alternative is to add some _additional_ functions, gated behind the parking_lot feature, for const construction of Semaphore, Mutex, etc.

I have no idea what this loom feature is all about and how it interacts with this feature request.

---

As far as I can tell, we would just need to add const to

• loom::std::parking_lot::Mutex::new
• util::LinkedList::new
• sync::batch_semaphore::Semaphore::new (conditionally)
• sync::Semaphore::new (conditionally)
• sync::Mutex::new (conditionally)
• sync::RwLock::new (conditionally)
• _possibly other users of Semaphore I haven’t found yet._ (Edit: for example Notify.)

[hawkw]

It would definitely be nice to have const fn constructors for the synchronization primitives when possible!

Some thoughts:

I propose that a few functions are made conditionally const, when the parking_lot feature is enabled. I’m not sure yet how nice this can work out in the documentation and how bad it would look in the source code though. If making e.g. Mutex::new conditionally const causes too much churn, an alternative is to add some _additional_ functions, gated behind the parking_lot feature, for const construction of Semaphore, Mutex, etc.

IMO, we should probably go with the second alternative, adding additional const fn constructors that are only present when the parking_lot feature.

This is mainly because I think it would make the API documentation significantly more obvious: since we build the docs.rs documentation with doc_cfg enabled, there would be a standard non-const new constructor, and then a const fn constructor with a "This is supported on feature="parking_lot" only." box on it. If we had a single constructor that's conditionally const, it wouldn't be nearly as obvious in the documentation that a feature flag is required to make it const. Also, I'm a little concerned that one constructor that may or may not be const could be a little hard for downstream code to reason about.

On the other hand, it does make the API surface a little uglier...

I have no idea what this loom feature is all about and how it interacts with this feature request.

loom is a permutation testing tool for concurrent Rust programs. It works by conditionally replacing synchronization primitives with simulated versions that can then be used to model all possible interleavings of concurrent operations. We use loom internally for testing Tokio's internals and synchronization primitives. Because loom's versions of synchronization primitives are bound to the currently running simulation when they are created, they cannot easily be made const.

However, we only replace the std and parking_lot synchronization primitives with loom's simulated versions when both cfg(loom) _and_ cfg(test) are enabled, so this only happens when running Tokio's loom tests. If a dependent crate has its own loom tests, it will still get the non-loom versions of Tokio APIs, since cfg(test) is enabled on a per-crate basis.

Therefore, if we add cfg attributes like

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

to all the const constructors, we _should_ be fine.

[steffahn]

Dammit, I'm only noticing this as everything is merged already...

@mental32 @Darksonn What was the rationale to not include RwLock?

[Darksonn]

None. Feel free to open a PR to add it.

[Darksonn]

Reopening as not all the types have const constructors yet.

[steffahn]

Looking a bit more into tokio::sync, const constructors for the types Semaphore and Notify seem reasonable too. _Edit:_ Just noticing sync::Semaphore was already my OP anyways.

[mental32]

Yikes I forgot to add it to RwLock, I can throw in a second PR covering the missed cases but I'll won't be able to get around to it for a few hours unfortunately.

[Darksonn]

but I won't be able to get around to it for a few hours unfortunately.

A few hours is very fast around here. I often measure turnaround time in weeks, sometimes months!

[steffahn]

Yikes I forgot to add it to RwLock, I can throw in a second PR covering the missed cases but I'll won't be able to get around to it for a few hours unfortunately.

Ah, thanks for the offer, but now I already created a PR myself ^^

@Darksonn feel free to take a look