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_manySemaphore::try_acquire_manySemaphore::acquire_many_ownedSemaphore::try_acquire_many_owned
Whereas the following Semaphore APIs use a usize count of permits:
Semaphore::newSemaphore::const_newSemaphore::available_permitsSemaphore::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]SemaphorePermitto hold ausizepermit count internally instead ofu32. I haven't confirmed, but I believe the size_ofSemaphorePermitwon'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 ausizepermit 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.