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
feat: merge Semaphore permits #4947
Comments
My main question is what should happen if the permits come from two different semaphores. |
Good point! I guess we could use a global, monotonic counter to tag each This is a more invasive change than I was hoping for though - I am unsure of the implications of this :( |
I mean, we wouldn't need a counter for that. The permit holds a pointer to the semaphore, so we can compare their address. However, perhaps a better solution is to call the function something like |
I have pushed a commit that prevents merging of unrelated Semaphore permits - the pointer check is a great idea. As for the name, I hold no strong opinions. I went with |
Done in #4948. |
Is your feature request related to a problem? Please describe.
When implementing resource limits using semaphores, it's common to
try_acquire()
a permit for each request, and drop it when the "work" is complete. However when the "work" involves batching together payloads and doing all the work at once (potentially some time after the actual request has completed), I find myself writing code like this:This is unfortunate, because we have to allocate a
Vec
for the permits, and grow it as more work is added in order to hold onto the permits. Maintaining thisVec
incurs both a memory, andadd_work()
latency cost.Describe the solution you'd like
Instead I'd love to be able to eliminate the
Vec
(and it's allocations) by mergingOwnedSemaphorePermit
instances together, with one instance holding all the permits:On the
OwnedSemaphorePermit
, this would look like:An identical impl would be added to the regular
SemaphorePermit
too.Describe alternatives you've considered
Vec<Permit>
as above, and pre-allocating space to avoid the re-allocations - this trades off (potentially wasted) memory for lack of grow latency, and can be hard to size correctly.Additional context
A
Semaphore
supports acquiring both owned and regular permits from the same instance, but this proposal doesn't support merging the two types together. For example, the following is not possible:The text was updated successfully, but these errors were encountered: