| msg197012 - (view) |
Author: Charles-François Natali (neologix) *  |
Date: 2013-09-05 16:46 |
| Here's a patch. |
|
|
| msg197014 - (view) |
Author: Richard Oudkerk (sbt) * |
Date: 2013-09-05 17:43 |
| 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. |
|
|
| msg197017 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-09-05 18:47 |
| New changeset 81f0c6358a5f by Charles-François Natali in branch 'default': Issue #18934: multiprocessing: use selectors module. http://hg.python.org/cpython/rev/81f0c6358a5f |
|
|
| msg197026 - (view) |
Author: Charles-François Natali (neologix) * |
Date: 2013-09-05 21:15 |
| 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. |
|
|
| msg197033 - (view) |
Author: Richard Oudkerk (sbt) * |
Date: 2013-09-05 22:08 |
| 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(). |
|
|
| msg197053 - (view) |
Author: Charles-François Natali (neologix) * |
Date: 2013-09-06 06:57 |
| > 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. |
|
|
| msg197092 - (view) |
Author: Charles-François Natali (neologix) * |
Date: 2013-09-06 19:01 |
| 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. |
|
|
| msg197097 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-09-06 19:17 |
| New changeset 14972c999e80 by Charles-François Natali in branch 'default': Issue #18934: Relax test_multiprocessing.test_invalid_handles a bit: we just http://hg.python.org/cpython/rev/14972c999e80 |
|
|
| msg197233 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-09-08 09:35 |
| New changeset 0e52b9f77dbf by Charles-François Natali in branch 'default': Issue #18934: Use poll/select-based selectors for multiprocessing.Connection, http://hg.python.org/cpython/rev/0e52b9f77dbf |
|
|