Repository navigation
Add 1.32x faster C implementation of asyncio.isfuture(). #77702
Description
| BPO | 33521 |
|---|---|
| Nosy | @pitrou, @vstinner, @asvetlov, @1st1, @jimmylai |
| PRs | |
| Files |
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:
assignee = None
closed_at = None
created_at = <Date 2018-05-15.15:31:32.301>
labels = ['expert-asyncio', '3.8', 'performance']
title = 'Add 1.32x faster C implementation of asyncio.isfuture().'
updated_at = <Date 2018-05-26.00:14:20.203>
user = 'https://github.com/jimmylai'bugs.python.org fields:
activity = <Date 2018-05-26.00:14:20.203>
actor = 'yselivanov'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2018-05-15.15:31:32.301>
creator = 'jimmylai'
dependencies = []
files = ['47593', '47603']
hgrepos = []
issue_num = 33521
keywords = ['patch']
message_count = 12.0
messages = ['316670', '316712', '317065', '317129', '317130', '317131', '317132', '317146', '317282', '317693', '317721', '317722']
nosy_count = 5.0
nosy_names = ['pitrou', 'vstinner', 'asvetlov', 'yselivanov', 'jimmylai']
pr_nums = ['6876']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue33521'
versions = ['Python 3.8']
Activity
jimmylai commentedon May 15, 2018
asyncio.isfuture called whenever ensure_future is called. Providing C implementation to make it fast.
jimmylai commentedon May 15, 2018
$./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%)
[-]Optimize asyncio.isfuture by providing C implementation[/-][+]Add 1.32x faster C implementation of asyncio.isfuture().[/+]vstinner commentedon May 18, 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 commentedon May 19, 2018
@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%)
pitrou commentedon May 19, 2018
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 commentedon May 19, 2018
@pitrou This change is part of optimization for asyncio.gather().
gather -> ensure_future -> isfuture/iscoroutine/isawaitable
We 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.
pitrou commentedon May 19, 2018
Then it would be nice to post benchmarks using asyncio.gather() (on something not too trivial perhaps).
jimmylai commentedon May 20, 2018
@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.
vstinner commentedon May 22, 2018
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 commentedon May 25, 2018
@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.py
vstinner commentedon May 26, 2018
I 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.
1st1 commentedon May 26, 2018
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).
iritkatriel commentedon Aug 10, 2022
Is it still worth doing this now that
gather()is "legacy"?