multiprocessing: use selectors module

[neologix]

BPO	18934
Nosy	@giampaolo
Files	selectors_multiprocessing.diff connection_selector.diff

^{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 = None
closed_at = <Date 2013-09-09.16:49:48.933>
created_at = <Date 2013-09-05.16:46:25.078>
labels = ['type-feature', 'library']
title = 'multiprocessing: use selectors module'
updated_at = <Date 2013-09-09.16:49:48.933>
user = 'https://bugs.python.org/neologix'

bugs.python.org fields:

activity = <Date 2013-09-09.16:49:48.933>
actor = 'neologix'
assignee = 'none'
closed = True
closed_date = <Date 2013-09-09.16:49:48.933>
closer = 'neologix'
components = ['Library (Lib)']
creation = <Date 2013-09-05.16:46:25.078>
creator = 'neologix'
dependencies = []
files = ['31616', '31629']
hgrepos = []
issue_num = 18934
keywords = ['patch', 'needs review']
message_count = 9.0
messages = ['197012', '197014', '197017', '197026', '197033', '197053', '197092', '197097', '197233']
nosy_count = 4.0
nosy_names = ['giampaolo.rodola', 'neologix', 'python-dev', 'sbt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue18934'
versions = ['Python 3.4']

[neologix]

Here's a patch.

[sbt]

LGTM.

But I would move "import selectors" in multiprocessing.connection to just before the definition of wait() for Unix. It is not needed on Windows and unnecessary imports slow down start up of new processes.

[python-dev]

New changeset 81f0c6358a5f by Charles-François Natali in branch 'default':
Issue bpo-18934: multiprocessing: use selectors module.
http://hg.python.org/cpython/rev/81f0c6358a5f

[neologix]

The test is failing on some (unstable) buildbots:
http://buildbot.python.org/all/builders/AMD64%20Solaris%2011%20%5BSB%5D%203.x/builds/1598/steps/test/logs/stdio
"""
======================================================================
FAIL: test_invalid_handles (test.test_multiprocessing_fork.TestInvalidHandle)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/cpython/buildslave/cc-32/3.x.snakebite-solaris11-amd64/build/Lib/test/_test_multiprocessing.py",
line 2962, in test_invalid_handles
    self.assertRaises((ValueError, OSError), conn.poll)
AssertionError: (<class 'ValueError'>, <class 'OSError'>) not raised by poll
"""

Basically, this test checks that calling poll() on an invalid
Connection (invalid FD) raises an OSError.
That's true for select, kqueue and epoll, but not for poll(), which
returns POLLNVAL.
I'm not sure about what to do here.

[sbt]

I remember wondering at one time why EPOLLNVAL did not exist, and realizing that closed fds are just silently unregistered by epoll().

I guess the issue is that some of the selectors indicate a bad fd on registration, and others do it when polled.

            On registration      On poll

----------------------------------------------------------------
SelectSelector No Raises OSError
PollSelector No No (EVENT_READ or EVENT_WRITE)
EpollSelector Raises OSError No
KqueueSelector ? ?

It would be easiest to relax the test, perhaps by just checking that conn.poll(0) raises or returns True.

Or maybe PollSelector.select() should raise OSError if POLLNVAL is indicated. That would match the behaviour of SelectSelector.select().

[neologix]

Richard Oudkerk added the comment:

I remember wondering at one time why EPOLLNVAL did not exist, and realizing that closed fds are just silently unregistered by epoll().

Exactly.

I guess the issue is that some of the selectors indicate a bad fd on registration, and others do it when polled.

            On registration      On poll

----------------------------------------------------------------
SelectSelector No Raises OSError
PollSelector No No (EVENT_READ or EVENT_WRITE)
EpollSelector Raises OSError No
KqueueSelector ? ?

Kqueue raises OSError upon registration. Not sure about poll().

It would be easiest to relax the test, perhaps by just checking that conn.poll(0) raises or returns True.

That's what I think too. Apparently, the test was added for this
issue: http://bugs.python.org/issue3321

Basically, the goal was to check that conn.poll() wouldn't crash if
passed an invalid FD (which can happen if you register a FD greater
than FD_SETSIZE in a fd_set).
So relaxing the check would still make sense.

Or maybe PollSelector.select() should raise OSError if POLLNVAL is indicated. That would match the behaviour of SelectSelector.select().

Yes, that would be a possibility. I'll have to give it some more thought.

[neologix]

I just realized that using DefaultSelector isn't optimal for Connection:
It's fine for forkserver, since it's a long lived process, but for
Connection.poll(), this means that it'll use epoll or kqueue: which
means allocating a new epoll/kqueue object for each conn.poll().
That's a couple syscalls more (epoll_create()/epoll_ctl()/close()),
but most important it uses an extra FD per connection.

The attached patch uses PollSelector if available, otherwise SelectSelector.

[python-dev]

New changeset 14972c999e80 by Charles-François Natali in branch 'default':
Issue bpo-18934: Relax test_multiprocessing.test_invalid_handles a bit: we just
http://hg.python.org/cpython/rev/14972c999e80

[python-dev]

New changeset 0e52b9f77dbf by Charles-François Natali in branch 'default':
Issue bpo-18934: Use poll/select-based selectors for multiprocessing.Connection,
http://hg.python.org/cpython/rev/0e52b9f77dbf