classification
Title: asyncio: Optimize loop.call_soon
Type: performance Stage: resolved
Components: asyncio Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: yselivanov Nosy List: asvetlov, gvanrossum, inada.naoki, ned.deily, python-dev, socketpair, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2016-11-03 18:29 by yselivanov, last changed 2017-07-28 03:34 by chris.jerdonek. This issue is now closed.

Files
File name Uploaded Description Edit
call_soon.patch yselivanov, 2016-11-03 18:35 review
call_soon2.patch yselivanov, 2016-11-03 19:04 moved another isinstance check from Handle to _check_callback review
call_soon3.patch yselivanov, 2016-11-03 19:21 address Guido's review review
Messages (12)
msg280002 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-03 18:29
loop.call_soon is the central function of asyncio.  Everything goes through it.

Current design of asyncio.loop.call_soon makes the following checks:

1. [debug mode] check that the loop is not closed
2. [debug mode] check that we are calling call_soon from the proper thread
3. [always] check that callback is not a coroutine or a coroutine function

Check #3 is very expensive, because it uses an 'isinstance' check.  Moreover, isinstance checks against an ABC, which makes the call even slower.

Attached patch makes check #3 optional and run only in debug mode.  This is a very safe patch to merge.

This makes asyncio another 17% (sic!) faster.  In fact it becomes as fast as curio for realistic streams benchmarks.
msg280004 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2016-11-03 18:42
The patch looks good.
IIRC haypo added the check because people called `.call_later()` with coroutine instead of callback very often.

But checking in debug mode looks very reasonable to me if it is so expensive.
msg280006 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-03 18:48
> IIRC haypo added the check because people called `.call_later()` with coroutine instead of callback very often.

We'll update asyncio docs in 3.6 with a tutorial to focus on coroutines (not on low-level event loop).  This should leave the loop API to advanced users.

> But checking in debug mode looks very reasonable to me if it is so expensive.

Exactly.
msg280013 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-03 19:31
I didn't review the patch, but I agree with the overall approach: expensive
checks can be made only in debug mode.

If people are concerned by the removal of the check by default, we should
repeat everywhere in the doc that async programming is hard and that the
debug mode saves a lot of time ;-)
msg280016 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-11-03 19:52
Patch 3 LGTM.
msg280017 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-03 20:51
call_soon3.patch: LGTM. It enhances error messages (fix the method name) and should make asyncio faster.
msg280018 - (view) Author: Марк Коренберг (socketpair) * Date: 2016-11-03 21:07
> haypo added the check because people called `.call_later()` with coroutine instead of callback very often

maybe make dirty hack and check hasattr(callback, 'send') ?
msg280019 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-03 21:11
> LGTM

Will commit this soon.

> maybe make dirty hack and check hasattr(callback, 'send') ?

If you schedule a coroutine object it will fail anyways, because it's not callable.
msg280026 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-03 22:11
New changeset 128ffe3c3eb9 by Yury Selivanov in branch '3.5':
Issue #28600: Optimize loop.call_soon().
https://hg.python.org/cpython/rev/128ffe3c3eb9

New changeset 4f570a612aec by Yury Selivanov in branch '3.6':
Merge 3.5 (issue #28600)
https://hg.python.org/cpython/rev/4f570a612aec

New changeset 46c3eede41a6 by Yury Selivanov in branch 'default':
Merge 3.6 (issue #28600)
https://hg.python.org/cpython/rev/46c3eede41a6
msg280027 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-03 22:12
Guido, Andrew, thanks for reviews!

I've fixed some unittests before committing the patch.
msg280030 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-11-03 22:26
PS. I noticed there are a few lines different between the "upstream" repo
and the 3.5 stdlib:

+            # Wake up the loop if the finalizer was called from
+            # a different thread.
+            self._write_to_self()

On Thu, Nov 3, 2016 at 3:12 PM, Yury Selivanov <report@bugs.python.org>
wrote:

>
> Changes by Yury Selivanov <yselivanov@gmail.com>:
>
>
> ----------
> resolution:  -> fixed
> stage: commit review -> resolved
> status: open -> closed
> type:  -> performance
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28600>
> _______________________________________
>
msg280031 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-11-03 22:29
+            # Wake up the loop if the finalizer was called from
+            # a different thread.
+            self._write_to_self()

Yeah, looks like shutdown_asyncgens somehow ended up in 3.5 branch (there's no harm in it being there).  I'll sync the branches.
History
Date User Action Args
2017-07-28 03:34:23chris.jerdoneksettitle: shutdown_asyncgens -> asyncio: Optimize loop.call_soon
2017-07-28 03:33:42chris.jerdoneksettitle: asyncio: Optimize loop.call_soon -> shutdown_asyncgens
2016-11-03 22:29:46yselivanovsetmessages: + msg280031
2016-11-03 22:26:39gvanrossumsetmessages: + msg280030
2016-11-03 22:12:50yselivanovsetstatus: open -> closed
type: performance
resolution: fixed
stage: commit review -> resolved
2016-11-03 22:12:18yselivanovsetmessages: + msg280027
2016-11-03 22:11:04python-devsetnosy: + python-dev
messages: + msg280026
2016-11-03 21:11:34yselivanovsetmessages: + msg280019
2016-11-03 21:07:27socketpairsetnosy: + socketpair
messages: + msg280018
2016-11-03 20:51:38vstinnersetmessages: + msg280017
2016-11-03 20:16:39ned.deilysetstage: commit review
2016-11-03 19:52:19gvanrossumsetmessages: + msg280016
2016-11-03 19:31:52vstinnersetmessages: + msg280013
2016-11-03 19:21:02yselivanovsetfiles: + call_soon3.patch
2016-11-03 19:04:38yselivanovsetfiles: + call_soon2.patch
2016-11-03 18:48:01yselivanovsetmessages: + msg280006
2016-11-03 18:42:12asvetlovsetmessages: + msg280004
2016-11-03 18:35:20yselivanovsetfiles: + call_soon.patch
keywords: + patch
2016-11-03 18:29:01yselivanovcreate