[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Yury Selivanov]

Yury Selivanov  added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Jimmy Lai  added the comment:

@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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[STINNER Victor]

STINNER Victor  added the comment:

> @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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Jimmy Lai  added the comment:

@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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Then it would be nice to post benchmarks using asyncio.gather() (on something 
not too trivial perhaps).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Jimmy Lai  added the comment:

@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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Antoine Pitrou]

Antoine Pitrou  added the comment:

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.

--
nosy: +pitrou

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Jimmy Lai  added the comment:

@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%)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[STINNER Victor]

STINNER Victor  added the comment:

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?

--
Added file: https://bugs.python.org/file47603/isfuture_benchmark2.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner
versions:  -Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Change by Jimmy Lai :


--
keywords: +patch
pull_requests: +6552
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33521] Add 1.32x faster C implementation of asyncio.isfuture().

[Jimmy Lai]

Jimmy Lai  added the comment:

$./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%)

--
title: Optimize asyncio.isfuture by providing C implementation -> Add 1.32x 
faster C implementation of asyncio.isfuture().
Added file: https://bugs.python.org/file47593/isfuture_benchmark.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com