msg279546 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-27 16:32 |
The attached patch implements asyncio.Task in C. Besides that, it also implements Argument Clinic for C Future.
Performance improvement on a simple echo server implementation using asyncio.streams:
Python Future & Task | C Future & Py Task | C Future & C Task
23K req/s | 26K | 30K
| ~10-15% boost | ~15%
Both Task and Future implemented in C, make asyncio programs up to 25-30% faster.
The patch is 100% backwards compatible. I modified asyncio tests to cross test Tasks and Futures implementations, i.e. to run Task+Future, Task+CFuture, CTask+Future, CTask+CFuture tests. No refleaks or other bugs. All uvloop functional tests pass without any problem.
Ned, Guido, are you OK if I merge this in 3.6 before beta 3? I'm confident that the patch is stable and even if something comes up we have time to fix it or even retract the patch. The performance boost is very impressive, and I can also make uvloop simpler.
|
msg279549 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-27 16:46 |
Also, with this patch uvloop becomes ~3-5% faster too.
|
msg279553 - (view) |
Author: Inada Naoki (methane) * |
Date: 2016-10-27 17:38 |
Wow! Great Job!
Yury, would you like to merge this before 3.6b3?
I'll look this as soon as possible.
(nit fix) Some modules doesn't sort imports in lexicography.
|
msg279554 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-27 17:40 |
> Yury, would you like to merge this before 3.6b3?
Yes!
> I'll look this as soon as possible.
Thanks a lot!
|
msg279560 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2016-10-27 18:35 |
If it's OK with Guido, it's OK with me for 360b3. (That's Monday.)
|
msg279564 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2016-10-27 19:18 |
Very impressive.
I've left a couple comments in rietveld though.
|
msg279568 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-10-27 20:39 |
I don't want to be the decider here. I don't have time to review the
code. I trust you all to do the right thing.
|
msg279569 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2016-10-27 20:48 |
I also trust Yury to do the right thing.
|
msg279570 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-27 21:16 |
Guido, Ned, thanks! Andrew already glanced through the code, let's see what Inada-san says.
I'm uploading an updated patch addressing Andrew's review.
|
msg279577 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 04:14 |
Uploading a new patch: sorted imports; added a unittest that exceptions in loop.call_soon aren't breaking Task.__init__.
|
msg279578 - (view) |
Author: Inada Naoki (methane) * |
Date: 2016-10-28 04:19 |
Why isfuture() is moved, and asyncio.coroutine uses
base_futures.isfuture() instead of futures.isfuture()?
|
msg279579 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 04:23 |
> Why isfuture() is moved, and asyncio.coroutine uses
> base_futures.isfuture() instead of futures.isfuture()?
Import cycles:
- `_asyncio` module is now being imported from `futures.py`
- and `_asyncio` is now imported from `tasks.py`;
- and `tasks.py` imports `futures.py` to have `isfuture`.
Long story short, I don't see a way of keeping `isfuture` in `futures.py`. The easiest option is to have it in a separate module.
|
msg279580 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 04:27 |
> - and `tasks.py` imports `futures.py` to have `isfuture`.
And tasks.py also imports coroutines.py (which was importing futures.py), making the cycle even worse. Anyways, I don't see a problem in moving a function or two that everybody uses into a separate module.
|
msg279613 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-10-28 16:53 |
New changeset db5ae4f6df8a by Yury Selivanov in branch '3.6':
Issue #28544: Implement asyncio.Task in C.
https://hg.python.org/cpython/rev/db5ae4f6df8a
New changeset 8059289b55c1 by Yury Selivanov in branch 'default':
Merge 3.6 (issue #28544)
https://hg.python.org/cpython/rev/8059289b55c1
|
msg279614 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 17:00 |
I've committed the patch with a few touchups:
* Applied code-review feedback.
* We now use AC for all methods, including Task._repr_info, Task._step, etc. See [1].
* I fixed the Task to be fully subclassable; users can override Task._step and Task._wakeup now. There are tests for ensuring that subclassing works the same for both Python and C implementations. See [2] and [3].
* I made it possible to override Future._schedule_subclass, as it was in Python 3.5. See [4].
Andrew, Inada-san, thank you for reviewing the patch! Please take a look at the patch updates referenced below. Feel free to re-open the issue if you see anything suspicious.
[1] https://github.com/1st1/cpython/commit/99adc35c94d76d78b8e666381436016c3c74a5ab
[2] https://github.com/1st1/cpython/commit/e294491c4cc7c92bc94ff12b4796c8ab12c40435
[3] https://github.com/1st1/cpython/commit/4d1c50c8987af124ce0bcccaea6bde82a60b59ee
[4] https://github.com/1st1/cpython/commit/efec03cbb4f81e78defd989d72c9bdae1122f50d
|
msg279618 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-10-28 17:17 |
New changeset fb7c439103b9 by Victor Stinner in branch '3.6':
Issue #28544: Fix _asynciomodule.c on Windows
https://hg.python.org/cpython/rev/fb7c439103b9
|
msg279629 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-10-28 21:38 |
Since asyncio becomes popular, IMO it is worth to mention this optimization at:
https://docs.python.org/dev/whatsnew/3.6.html#optimizations
Can you please do it Yury?
|
msg279630 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 21:41 |
> Can you please do it Yury?
Yes, Elvis and I will be the editors of What's New this year anyways ;)
|
msg279632 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-10-28 22:05 |
Python 3.6 & default don't compile on Windows because of the change:
http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x/builds/2795/steps/compile/logs/stdio
"D:\buildarea\3.x.bolen-windows8\build\PCbuild\_asyncio.vcxproj" (Build target) (15) ->
(Link target) ->
_asynciomodule.obj : error LNK2019: unresolved external symbol _PyDict_Pop referenced in function task_step [D:\buildarea\3.x.bolen-windows8\build\PCbuild\_asyncio.vcxproj]
_asynciomodule.obj : error LNK2019: unresolved external symbol _PyGen_Send referenced in function task_step_impl [D:\buildarea\3.x.bolen-windows8\build\PCbuild\_asyncio.vcxproj]
D:\buildarea\3.x.bolen-windows8\build\PCBuild\amd64\_asyncio_d.pyd : fatal error LNK1120: 2 unresolved externals [D:\buildarea\3.x.bolen-windows8\build\PCbuild\_asyncio.vcxproj]
|
msg279635 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-10-28 22:49 |
New changeset 635f5854cb3e by Yury Selivanov in branch '3.6':
Issue #28544: Fix compilation of _asynciomodule.c on Windows
https://hg.python.org/cpython/rev/635f5854cb3e
New changeset 9d95510dc203 by Yury Selivanov in branch 'default':
Merge 3.6 (issue #28544)
https://hg.python.org/cpython/rev/9d95510dc203
|
msg279636 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-28 22:52 |
> Python 3.6 & default don't compile on Windows because of the change:
Pushed a fix for that.
|
msg279638 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-10-28 23:01 |
New changeset db7bcd92cf85 by Yury Selivanov in branch '3.6':
Issue #28544: Pass `PyObject*` to _PyDict_Pop, not `PyDictObject*`
https://hg.python.org/cpython/rev/db7bcd92cf85
New changeset 60b6e820abe5 by Yury Selivanov in branch 'default':
Merge 3.6 (issue #28544)
https://hg.python.org/cpython/rev/60b6e820abe5
|
msg279645 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2016-10-29 02:17 |
Looks like that Windows buildbot is green now. Closing.
|
msg279657 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-10-29 07:12 |
New changeset 950fbd75223b by Victor Stinner in branch '3.6':
Issue #28544: Fix inefficient call to _PyObject_CallMethodId()
https://hg.python.org/cpython/rev/950fbd75223b
|
msg279805 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2016-10-31 17:00 |
Just an FYI, for testing code that has both pure Python and C versions you can follow the advice in https://www.python.org/dev/peps/pep-0399/#details . And if you want to really simplify things you start down the road of what test_importlib uses: https://github.com/python/cpython/blob/master/Lib/test/test_importlib/util.py#L81 .
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:38 | admin | set | github: 72730 |
2016-10-31 17:00:26 | brett.cannon | set | nosy:
+ brett.cannon messages:
+ msg279805
|
2016-10-29 07:12:38 | python-dev | set | messages:
+ msg279657 |
2016-10-29 02:17:28 | yselivanov | set | status: open -> closed resolution: fixed messages:
+ msg279645
|
2016-10-28 23:01:53 | python-dev | set | messages:
+ msg279638 |
2016-10-28 22:52:23 | yselivanov | set | messages:
+ msg279636 |
2016-10-28 22:49:17 | python-dev | set | messages:
+ msg279635 |
2016-10-28 22:05:35 | vstinner | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg279632
|
2016-10-28 21:41:41 | yselivanov | set | messages:
+ msg279630 |
2016-10-28 21:38:42 | vstinner | set | messages:
+ msg279629 |
2016-10-28 17:17:23 | python-dev | set | messages:
+ msg279618 |
2016-10-28 17:00:43 | yselivanov | set | status: open -> closed resolution: fixed messages:
+ msg279614
stage: patch review -> resolved |
2016-10-28 16:53:37 | python-dev | set | nosy:
+ python-dev messages:
+ msg279613
|
2016-10-28 04:27:46 | yselivanov | set | messages:
+ msg279580 |
2016-10-28 04:23:10 | yselivanov | set | messages:
+ msg279579 |
2016-10-28 04:19:59 | methane | set | messages:
+ msg279578 |
2016-10-28 04:14:11 | yselivanov | set | files:
+ ctask3.patch
messages:
+ msg279577 |
2016-10-27 21:16:37 | yselivanov | set | files:
+ ctask2.patch
messages:
+ msg279570 |
2016-10-27 20:48:07 | ned.deily | set | messages:
+ msg279569 |
2016-10-27 20:39:58 | gvanrossum | set | messages:
+ msg279568 |
2016-10-27 19:18:33 | asvetlov | set | messages:
+ msg279564 |
2016-10-27 18:35:16 | ned.deily | set | messages:
+ msg279560 |
2016-10-27 17:40:51 | yselivanov | set | messages:
+ msg279554 |
2016-10-27 17:38:58 | methane | set | messages:
+ msg279553 |
2016-10-27 16:46:59 | yselivanov | set | messages:
+ msg279549 |
2016-10-27 16:35:54 | Elvis.Pranskevichus | set | nosy:
+ Elvis.Pranskevichus
|
2016-10-27 16:33:11 | yselivanov | create | |