Repository navigation
Add 1.32x faster C implementation of asyncio.isfuture(). #77702
Description
Activity
jimmylai commented
on May 15, 2018 jimmylaimannequinMannequinAuthorasyncio.isfuture called whenever ensure_future is called. Providing C implementation to make it fast.
jimmylai commented
on May 15, 2018 jimmylaimannequinMannequinAuthor$./python.exe isfuture_benchmark.py -o isfuture_optimized.json
$ ./python.exe -m perf compare_to isfuture_original.json isfuture_optimized.json
Mean +- std dev: [isfuture_original] 7.16 ms +- 0.23 ms -> [isfuture_optimized] 5.41 ms +- 0.25 ms: 1.32x faster (-24%)- changed the title
[-]Optimize asyncio.isfuture by providing C implementation[/-][+]Add 1.32x faster C implementation of asyncio.isfuture().[/+]on May 15, 2018 About the benchmark: you should not loop inside the function. I propose a variant of the benchmark: isfuture_benchmark2.py.
Jimmy: Would you mind to run this variant on your PR?
jimmylai commented
on May 19, 2018 jimmylaimannequinMannequinAuthor@vstinner Thanks for the new benchmark, it provides more detailed wins:
It's 1.64x faster for future object and 1.23x faster for non-future object.
$ ./python.exe -m perf compare_to isfuture_original_2.json isfuture_optimized_2.json
future: Mean +- std dev: [isfuture_original_2] 224 ns +- 8 ns -> [isfuture_optimized_2] 135 ns +- 2 ns: 1.66x faster (-40%)
task: Mean +- std dev: [isfuture_original_2] 224 ns +- 6 ns -> [isfuture_optimized_2] 137 ns +- 3 ns: 1.64x faster (-39%)
regular_func: Mean +- std dev: [isfuture_original_2] 443 ns +- 5 ns -> [isfuture_optimized_2] 361 ns +- 5 ns: 1.23x faster (-18%)
str: Mean +- std dev: [isfuture_original_2] 449 ns +- 15 ns -> [isfuture_optimized_2] 360 ns +- 12 ns: 1.25x faster (-20%)What is the impact on a regular application? The fact that a micro-benchmark becomes X% faster isn't very interesting itself, especially as the original function, at less than 1µs per call, is already very fast.
jimmylai commented
on May 19, 2018 jimmylaimannequinMannequinAuthor@pitrou This change is part of optimization for asyncio.gather().
gather -> ensure_future -> isfuture/iscoroutine/isawaitableWe need C implementation for all those function to make gather really efficient for large scale application (e.g. Instagram)
Gather is really slow and cost ~2% CPU on our server.The same optimization approach has been apply on other ciritcal asyncio modules, e.g. Future, get_event_loop, etc.
Then it would be nice to post benchmarks using asyncio.gather() (on something not too trivial perhaps).
jimmylai commented
on May 20, 2018 jimmylaimannequinMannequinAuthor@pitrou We'll measure the wins of gather when we implement it in C. Before that, we need to get all helpers ready in C.
@pitrou We'll measure the wins of gather when we implement it in C. Before that, we need to get all helpers ready in C.
You don't have to provide _asyncio.isfuture() (in C) to implement gather() in C.
If your goal is to optimize gather(), write a change to only implement gather() no?
What's the point of optimizing isfuture() is gather() is implemented in C and doesn't call the Python implementation anymore?
Implement isfuture() is C and expose it as _asyncio.isfuture() are two different things.
jimmylai commented
on May 25, 2018 jimmylaimannequinMannequinAuthor@vstinner
In general, we would like to make all asyncio common functions efficient with C implementation because CPython asyncio overhead is very expensive.
In our application, overall it costs about 10% global CPU instructions after we used UVLoop (it's much worse when use default event loop).
gather() is only one of the high level function bottleneck. To make CPU overhead not a concern for asyncio user, we should make isfuture in C because it's called by many other event loop functions, e.g. in asyncio/tasks.py, asyncio/coroutines.py, asyncio/base_events.pyI disagree with your rationale. At least, it is not how we decide to
optimize something or not in Python. Python is first made of people who
will have to maintain the code for the next 5 years if not longer. C code
is harder to maintain than Python code.A significant speedup on a microbenchmark is nice, but asyncio still lacks
a more general benchmark suite...I have no opinion on this specific optimisation.
IMO this particular patch is OK and should go in. I'll take another look at the PR in a few days (and will run some benchmarks myself before making the decision).
Is it still worth doing this now that
gather()is "legacy"?
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: