Patchwork [1,of,4] util: add a simple poll utility

login
register
mail settings
Submitter Pierre-Yves David
Date June 3, 2015, 8:26 p.m.
Message ID <0521446e1c5934bf377d.1433363190@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/9461/
State Superseded
Commit c2ec81891502662ebe2d3ad987bd0ea60ab1b60f
Headers show

Comments

Pierre-Yves David - June 3, 2015, 8:26 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1432162805 18000
#      Wed May 20 18:00:05 2015 -0500
# Node ID 0521446e1c5934bf377d8c6bd8a48baad6b57ae1
# Parent  d3b8690c4ecc2a60b8edd9f632f77824284b6027
util: add a simple poll utility

We'll use it to detect when the a sshpeer have server output to be displayed.

The implementation is super basic because all case support is not the focus of
this series.
Matt Mackall - June 3, 2015, 10:17 p.m.
On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1432162805 18000
> #      Wed May 20 18:00:05 2015 -0500
> # Node ID 0521446e1c5934bf377d8c6bd8a48baad6b57ae1
> # Parent  d3b8690c4ecc2a60b8edd9f632f77824284b6027
> util: add a simple poll utility
> 
> We'll use it to detect when the a sshpeer have server output to be displayed.
> 
> The implementation is super basic because all case support is not the focus of
> this series.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -16,11 +16,11 @@ hide platform-specific details from the 
>  import i18n
>  _ = i18n._
>  import error, osutil, encoding, parsers
>  import errno, shutil, sys, tempfile, traceback
>  import re as remod
> -import os, time, datetime, calendar, textwrap, signal, collections
> +import os, time, datetime, calendar, textwrap, signal, collections, sys, select

Hmm, is it really possible that util doesn't import sys? Very nearby
signs point to no.

> +def poll(fds):
> +    """block until something happened on any filedescriptors
> +
> +    This is a generic helper that will check for any activity
> +    (read, write.  exception). return the list of touched file.
> +
> +    In unsupported case raise a NotImplementedError"""
> +    if os.name == 'nt': # we do not support windows yet.
> +        raise NotImplementedError()
> +    if any(sys.maxint <= f for f in fds):
> +        raise NotImplementedError()
> +    res = select.select(fds, fds, fds)
> +    return sorted(list(set(sum(res, []))))
> +

Please use posix.py and windows.py, rather than putting conditionals in
util.
Matt Mackall - June 3, 2015, 11:03 p.m.
On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote:
> +    if any(sys.maxint <= f for f in fds):
> +        raise NotImplementedError()

Also, I'm not sure why you're testing against maxint. The actual limit
is typically something like 2048, defined in FD_SETSIZE. And the reason
for the limit is that every possible file descriptor up to that limit is
represented as a bit in the input field. A bit vector with 2^64 bits is
indeed too large.

Modern interfaces like poll/epoll don't try to pass in all the
descriptors at once and thus don't run into these limits.
Pierre-Yves David - June 4, 2015, 12:54 a.m.
On 06/03/2015 04:03 PM, Matt Mackall wrote:
> On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote:
>> +    if any(sys.maxint <= f for f in fds):
>> +        raise NotImplementedError()
>
> Also, I'm not sure why you're testing against maxint. The actual limit
> is typically something like 2048, defined in FD_SETSIZE. And the reason
> for the limit is that every possible file descriptor up to that limit is
> represented as a bit in the input field. A bit vector with 2^64 bits is
> indeed too large.

Neat, Augie and I were looking for such variable with no luck. Python 
store this in TYPES.FD_SETSIZE. But is also turn out that Python raise a 
ValueError if you give a too large file descriptor to select. So we can 
just catch that. V2 on the way.

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -16,11 +16,11 @@  hide platform-specific details from the 
 import i18n
 _ = i18n._
 import error, osutil, encoding, parsers
 import errno, shutil, sys, tempfile, traceback
 import re as remod
-import os, time, datetime, calendar, textwrap, signal, collections
+import os, time, datetime, calendar, textwrap, signal, collections, sys, select
 import imp, socket, urllib
 import gc
 
 if os.name == 'nt':
     import windows as platform
@@ -327,10 +327,24 @@  class bufferedinputpipe(object):
             self._eof = True
         else:
             # inefficient add
             self._buffer.append(data)
 
+def poll(fds):
+    """block until something happened on any filedescriptors
+
+    This is a generic helper that will check for any activity
+    (read, write.  exception). return the list of touched file.
+
+    In unsupported case raise a NotImplementedError"""
+    if os.name == 'nt': # we do not support windows yet.
+        raise NotImplementedError()
+    if any(sys.maxint <= f for f in fds):
+        raise NotImplementedError()
+    res = select.select(fds, fds, fds)
+    return sorted(list(set(sum(res, []))))
+
 def popen2(cmd, env=None, newlines=False):
     # Setting bufsize to -1 lets the system decide the buffer size.
     # The default for bufsize is 0, meaning unbuffered. This leads to
     # poor performance on Mac OS X: http://bugs.python.org/issue4194
     p = subprocess.Popen(cmd, shell=True, bufsize=-1,