tokio
tokio copied to clipboard
Semaphore::acquire_many API with `usize` count
Is your feature request related to a problem? Please describe.
The following Semaphore
APIs use a u32
count of permits:
-
Semaphore::acquire_many
-
Semaphore::try_acquire_many
-
Semaphore::acquire_many_owned
-
Semaphore::try_acquire_many_owned
Whereas the following Semaphore APIs use a usize
count of permits:
-
Semaphore::new
-
Semaphore::const_new
-
Semaphore::available_permits
-
Semaphore::add_permits
This incongruity in the API is awkward, but it also prohibits a key usecase for the Semaphore
type: protecting access to memory, e.g. memory leases. In fact the PR which added the u32
APIs mentions this usecase, but presumably the author didn't need to support leases of more than 4GiB (#2607).
Describe the solution you'd like
- Change
[Owned]SemaphorePermit
to hold ausize
permit count internally instead ofu32
. I haven't confirmed, but I believe the size_ofSemaphorePermit
won't change due to padding, so hopefully this won't have any performance impact. - Add four new methods replacing each permutation of
[try_]acquire_many[_owned]
, and which take ausize
permit count. To kick off the painting, I propose potential names below. - [optional] Deprecate
Semaphore::[try_]acquire_many[_owned]
.
Describe alternatives you've considered
For applications which need to acquire more than 2^32 permits there really isn't a way to use the current semaphore API - it's not correct to acquire permits in a loop due to risk of deadlock.
Additional context
Potential names for the new methods:
-
[try_]acquire_many_permits[_owned]
-
[try_]acquire_permits[_owned]
-
[try_]acquire_n[_owned]
I've added a PR with the proposed new methods here: https://github.com/tokio-rs/tokio/pull/4447. I went with the names [try_]acquire_permits[_owned]
, but definitely open to changing that. I did not add deprecation tags to the original methods, but can if that's the consensus.
It's unfortunate that there's a mismatch in the types, but I'm not convinced its worth fixing it. Note that on 32-bit platforms, the maximum number of permits isn't 2^32, but rather 2^29. Similarly, on 64-bit, the maximum is 2^61.
@Darksonn with the API as it is, it's not possible to use Semaphore
to implement memory leases with support for leases > 4GiB. Memory leases of this style are a common technique data analysis systems, warehousing, query execution, etc. For these usecases a 4GiB limit is orders of magnitude too low. The 2^61 limit is not an issue since no practical system is approaching that much memory, and x86-64 is typically limited to 52 bytes of addressable space anyway.
WRT the cost of fixing it, the semaphore internals are already in terms of usize
so it's not a major change.
It's situationally possible to extend the usefulness of the current Semaphore
type by mapping a lease to a page of memory instead of a byte, yielding a leases of up to 2 ^ 44 = 16 TiB, which is fine for most systems now, but perhaps only for a few more years.
:+1: - am using a semaphore mapping to byte count, would be nice to not have the upper u32::MAX >> 3 ceiling.
In this application: https://github.com/aaronriekenberg/rust-parallel/tree/0.1.20
I use a Semaphore to limit the number of parallel commands run.
At the end I call acquire_many
to wait for all command tasks to complete: https://github.com/aaronriekenberg/rust-parallel/blob/0.1.20/src/command.rs#L212
Using u32
for --jobs/-j
command line argument to match type of acquire_many
: https://github.com/aaronriekenberg/rust-parallel/blob/0.1.20/src/command_line_args.rs#L15-L17
This requires conversion from num_cpus::get()
which returns usize
. Also requires conversion when creating the Semaphore: https://github.com/aaronriekenberg/rust-parallel/blob/0.1.20/src/command.rs#L79-L82
If acquire_many
took a usize
parameter I could use usize
for --jobs/-j
parameter. Would eliminate conversions and be cleaner for this use case.