classification
Title: asyncio: break some cycles
Type: performance Stage:
Components: asyncio Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, haypo, martius, python-dev, yselivanov
Priority: normal Keywords: patch

Created on 2015-01-09 17:16 by martius, last changed 2015-01-15 08:24 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
break-some-cycles.diff martius, 2015-01-09 17:16
break-selector-map-cycle.diff martius, 2015-01-12 10:16
Messages (11)
msg233770 - (view) Author: Martin Richard (martius) * Date: 2015-01-09 17:16
Hi,

I would like to submit 3 trivial modifications which break a cycle each. It is not much, but those three cycles caused a lot of objects to be garbage collected. They can now be freed using the reference counting mechanism, and therefore, reduce the latency that may be involved by the work of the garbage collector in a long living process.

In asyncio/base_subprocess.py:
WriteSubprocessPipeProto.proc is a reference to a BaseSubprocessTransport object, which holds a reference to the WriteSubprocessPipeProto in self._protocol.

I break the cycle in the protocol at the end of connection_lost().

In asyncio/futures.py:
wrap_future() defines a lambda which uses a variable defined in the function, therefore creating a closure, referencing the wrap_future() function and creating a cycle.

In the (really trivial) patch, the lambda uses the argument "future" instead of the "fut" variable defined in a closure. The closure is not needed anymore.

This single cycle is very common, because caused when one uses getaddrinfo().

In asyncio/selectors.py:
_BaseSelectorImpl._map keeps a reference to the _SelectorMapping object, which also references the selector with _SelectorMapping._selector.

The reference to the map in the selector is cleared once the selector is closed.
msg233772 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2015-01-09 17:34
All three changes look good to me. The selectors.py fix should be applied to CPython first; the others to Tulip first.
msg233781 - (view) Author: Roundup Robot (python-dev) Date: 2015-01-09 20:36
New changeset 376c5398f28d by Victor Stinner in branch '3.4':
Issue #23209: Break some reference cycles in asyncio. Patch written by Martin
https://hg.python.org/cpython/rev/376c5398f28d
msg233782 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-01-09 20:37
Hi Martin, thanks for the patch. It looks good to me. I applied it to Tulip, Python 3.4 and Python 3.5.
msg233783 - (view) Author: Roundup Robot (python-dev) Date: 2015-01-09 20:58
New changeset 7438f2e30908 by Victor Stinner in branch '3.4':
Issue #23209: Revert change on selectors, test_selectors failed.
https://hg.python.org/cpython/rev/7438f2e30908

New changeset 27cbc877447b by Victor Stinner in branch 'default':
(Merge 3.4) Issue #23209: Revert change on selectors, test_selectors failed.
https://hg.python.org/cpython/rev/27cbc877447b
msg233784 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-01-09 21:05
Ooops, test_selectors fails because get_key() raises "TypeError: 'NoneType' object is not subscriptable" when the selector is closed.

A different fix should be written.

I'm now using tox to run the Tulip test suite, I'm surprised that I didn't notice the failure. It looks like test_selectors is not executed. runtests.py searchs for test classes with a name ending with "Tests". I will fix that.
msg233880 - (view) Author: Martin Richard (martius) * Date: 2015-01-12 10:16
I updated the selector patch so BaseSelector.get_key() raises KeyError if the mapping is None. All the (non skipped) tests in test_selectors.py passed.

Anyway, if there is an other problem with freeing the mapping object (I don't know, maybe "reopening" a loop may be considered?) this patch can probably be dropped. Since this cycle is broken when the loop is closed, the objects will likely be collected once the program terminates.
msg233882 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-01-12 11:06
I opened the issue #23225 "selectors: raise an exception if the selector is closed" which is a different approach (but it should also fix the reference cycle, I kept the "self._map = None" change).
msg233909 - (view) Author: Roundup Robot (python-dev) Date: 2015-01-13 09:01
New changeset 1544bdc409be by Victor Stinner in branch '3.4':
Issue #23209, #23225: selectors.BaseSelector.close() now clears its internal
https://hg.python.org/cpython/rev/1544bdc409be

New changeset 6e7403bc906f by Victor Stinner in branch 'default':
Issue #23209, #23225: selectors.BaseSelector.get_key() now raises a
https://hg.python.org/cpython/rev/6e7403bc906f
msg233913 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-01-13 09:07
I closed the issue #23225, so I'm also closing this issue. Thanks again Martin.
msg234063 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-01-15 08:24
I noticed that _ProactorBasePipeTransport doesn't clear its reference to the socket when it is closed. I changed this in the following commit (now merged in Python 3.4 & 3.5):
https://code.google.com/p/tulip/source/detail?r=61ce7def97272ab1a6488545dc392160c2fbe316
History
Date User Action Args
2015-01-15 08:24:41hayposetmessages: + msg234063
2015-01-13 09:07:44hayposetstatus: open -> closed
resolution: fixed
messages: + msg233913
2015-01-13 09:01:51python-devsetmessages: + msg233909
2015-01-12 11:06:32hayposetmessages: + msg233882
2015-01-12 10:16:49martiussetfiles: + break-selector-map-cycle.diff

messages: + msg233880
2015-01-09 21:05:27hayposetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg233784
2015-01-09 20:58:08python-devsetmessages: + msg233783
2015-01-09 20:37:20hayposetstatus: open -> closed
resolution: fixed
messages: + msg233782
2015-01-09 20:36:51python-devsetnosy: + python-dev
messages: + msg233781
2015-01-09 17:34:38gvanrossumsetmessages: + msg233772
2015-01-09 17:16:47martiussetnosy: + gvanrossum, haypo, yselivanov

type: performance
components: + asyncio
versions: + Python 3.4
2015-01-09 17:16:15martiuscreate