mjpieters / aiolimiter

An efficient implementation of a rate limiter for asyncio.
https://aiolimiter.readthedocs.io/en/latest/
MIT License
512 stars 22 forks source link

Faster acquire #218

Open schoennenbeck opened 8 months ago

schoennenbeck commented 8 months ago

Acquire now only waits the minimum amount of time needed for enough drip to occur. See: https://github.com/mjpieters/aiolimiter/issues/217

sonarcloud[bot] commented 8 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

schoennenbeck commented 5 months ago

I added a test which is basically just the example code I put in the issue. Feel free to suggest a different method of testing this; I am not sure this is the most elegant way to do it.

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

mk-armah commented 2 months ago

Whats the status of this ?

bpgould commented 2 months ago

once @mjpieters approves changes it can be merged. +1 from me

schoennenbeck commented 2 weeks ago

@mjpieters Could we get this merged?

sveinugu commented 3 days ago

It seems this change messes up the order of the self._waiters queue:

import asyncio

from aiolimiter import AsyncLimiter

rate_limit = AsyncLimiter(5, 1)

async def some_coroutine(request_num: int):
    await rate_limit.acquire()
    print(f'Request number: {request_num}')

async def main():
    tasks = [some_coroutine(i) for i in range(10)]
    await asyncio.gather(*tasks)

asyncio.run(main())

Output from current code:

Request number: 0
Request number: 1
Request number: 2
Request number: 3
Request number: 4
Request number: 5
Request number: 6
Request number: 7
Request number: 8
Request number: 9

Output after PR (e.g.):

Request number: 0
Request number: 1
Request number: 2
Request number: 3
Request number: 4
Request number: 7
Request number: 6
Request number: 8
Request number: 9
Request number: 5
schoennenbeck commented 2 days ago

@sveinugu That's interesting. My best guess would be that this is due to some floating point inaccuracy. I'll have a look at it. Though to be fair: The original implementation was never "first come first served" to begin with. It just so happens that in very simple examples in ends up this way.