Repository navigation
Implement asyncio Future in C to improve performance #70269
Description
| BPO | 26081 |
|---|---|
| Nosy | @gvanrossum, @vstinner, @giampaolo, @ned-deily, @methane, @serhiy-storchaka, @1st1, @mpaolini, @RemiCardona |
| PRs | Misc/NEWS so that it is managed by towncrier #552 |
| 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 = 'https://github.com/1st1'
closed_at = <Date 2016-11-09.00:06:18.117>
created_at = <Date 2016-01-11.17:23:42.941>
labels = ['expert-asyncio', '3.7', 'performance']
title = 'Implement asyncio Future in C to improve performance'
updated_at = <Date 2017-03-31.16:36:12.613>
user = 'https://github.com/1st1'bugs.python.org fields:
activity = <Date 2017-03-31.16:36:12.613>
actor = 'dstufft'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2016-11-09.00:06:18.117>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2016-01-11.17:23:42.941>
creator = 'yselivanov'
dependencies = []
files = ['41579', '43670', '44081', '44531', '44544', '44547', '44670', '44993', '45003', '45012', '45023']
hgrepos = []
issue_num = 26081
keywords = ['patch']
message_count = 50.0
messages = ['257984', '269773', '270037', '270055', '270068', '270069', '270070', '270175', '270183', '270221', '270232', '270924', '270928', '272242', '272245', '272262', '272278', '272285', '272327', '272347', '272498', '273802', '275658', '275729', '275734', '275740', '275903', '275904', '275905', '275906', '276494', '276504', '276505', '276513', '278204', '278206', '278224', '278227', '278243', '278294', '278325', '278328', '278348', '278357', '278365', '278366', '278368', '278370', '280359', '280363']
nosy_count = 10.0
nosy_names = ['gvanrossum', 'vstinner', 'giampaolo.rodola', 'ned.deily', 'methane', 'python-dev', 'serhiy.storchaka', 'yselivanov', 'mpaolini', 'RemiCardona']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue26081'
versions = ['Python 3.6', 'Python 3.7']
Activity
1st1 commentedon Jan 11, 2016
Some info on this: python/asyncio#282 (comment) Long story short, Future implemented in C can speedup some asyncio code up to 25%.
I'm attaching a patch with a WIP implementation. There are some failing assertions deep in GC, which I need to track down. 'Future.remove_done_callback' still needs to be properly implemented.
gvanrossum commentedon Jul 3, 2016
We should really try to get this into 3.6.
--Guido (mobile)
methane commentedon Jul 9, 2016
I'm working on this. Some bugs are fixed, but doesn't pass tests for now.
methane#5
methane commentedon Jul 9, 2016
Passing all tests now.
Yury, could you explain what the comment "This isn't a Future class; it's a BaseFuture" means?
Should it be "_futures.Future" or "_futures.BaseFuture"?
methane commentedon Jul 10, 2016
Should I send pull request to github.com/python/asyncio?
Or should I post patch here?
1st1 commentedon Jul 10, 2016
Thanks a lot! I couldn't find time to finish this myself. I can definitely help you and review the patch once it's ready.
Unfortunately I don't remember :(
Please post it here. AFAIK we haven't yet transitioned to the GitHub.
methane commentedon Jul 10, 2016
OK. Here is current version.
methane commentedon Jul 11, 2016
In my patch, test_asyncio runs against C version Future.
I saw how test_json tests against C version and pure Python version.
But test_asyncio is more larger than test_json.
Before working on it, could someone give me idea to run whole test_asyncio
with and without C version Future easily?
And, which is master repository of asyncio? github? or hg.python.org?
If github, can I send separated pull request changing test_asyncio after
this patch is merged?
1st1 commentedon Jul 11, 2016
asyncio uses loop.create_future() to create sockets. I'd suggest you to create two base test classes: one that monkeypatches loop.create_future to return pure python Future in its setUp method; an another, that makes create_future to return a C version of the Future.
The derive some unittests from those base classes (which will effectively double the number of tests).
The master repo for asyncio is github, but since the C version won't be a part of asyncio (it will be checked in only in CPython source tree), I think it's fine to continue the work here, on bugs.python.org.
methane commentedon Jul 12, 2016
Thanks. I'll working on test_asyncio in next few days.
methane commentedon Jul 12, 2016
windows_events.py has some classes extends futures.Future.
Task extends Future.
There are some
isinstance(future, futures.Future).So monkeypatching
baseevent.create_futureseems not enough.I want a way to completely reload asyncio and test_asyncio packages with and without C future.
mpaolini commentedon Jul 21, 2016
THe guys developing uvloop say their implementation is already quite fast [1]. Is it worth integrating it?
[1] https://github.com/MagicStack/uvloop
30 remaining items
methane commentedon Oct 9, 2016
I've committed the patch with trivial fixes (adding curly braces to if statements).
And I'm sorry, I committed with wrong issue number.
https://hg.python.org/cpython/rev/678424183b38 (3.6)
https://hg.python.org/cpython/rev/f8815001a390 (default)
I fixed NEWS entry already.
methane commentedon Oct 9, 2016
I close this issue for now.
Further improvements can be new issue.
1st1 commentedon Oct 9, 2016
Thank you, INADA! Next task -- optimize asyncio.Task in C in 3.7. Another 10-15% performance improvement.
1st1 commentedon Oct 9, 2016
I mean another optimization possibility.
methane commentedon Oct 9, 2016
How about changing module name?
_asyncio_speedup for example.
1st1 commentedon Oct 9, 2016
Yes, I think it's a good idea.
1st1 commentedon Nov 8, 2016
This patch introduced multiple refleaks in test_asyncgen.
python-dev commentedon Nov 9, 2016
New changeset 345904bd0456 by Yury Selivanov in branch '3.6':
Issue bpo-26081: Fix refleak in _asyncio.Future.__iter__().throw.
https://hg.python.org/cpython/rev/345904bd0456
New changeset b977775aa07d by Yury Selivanov in branch 'default':
Merge 3.6 (issue bpo-26081)
https://hg.python.org/cpython/rev/b977775aa07d