Python popenX() slowness on AIX?

[allenjo5 at mail.northgrum.com]

Stefaan A Eeckels wrote:
> On 21 Nov 2006 13:02:14 -0800
> allenjo5 at mail.northgrum.com wrote:
>
> > The fact that it does this in Python code instead of C is the main
> > cause of the slowness.  So, unless Python is changed to do this in C,
> > it's always going to be slow on AIX :-(
>
> I guess that the reason it's slow is that there are many descriptors to
> try and close. Reducing them using ulimit -n could improve the speed.
>
> AIX has a fcntl command to close all open file descriptors from a
> descriptor onwards:
>
> fcntl(3, F_CLOSEM);
>
> This of course should be used instead of the loop:
>
> import sys
> import fcntl
>
> __all__ = ["popen2", "popen3", "popen4"]
>
> ...
>
>     def _run_child(self, cmd):
>         if isinstance(cmd, basestring):
>             cmd = ['/bin/sh', '-c', cmd]
>         try:
>             os.fcntl(3, 10, 0)
>         except OSError:
>             pass
>         try:
>             os.execvp(cmd[0], cmd)
>         finally:
>             os._exit(1)
>
> 10 happens to be the value of F_CLOSEM (from /usr/include/fcntl.h).
> I've currently no access to an AIX system with Python, but it could be
> worth trying.

Yes, very much worth it.  F_CLOSEM is _so_ much better than the loop,
even in C.   Using your brilliant suggestion, I now have a simple patch
to the python source that implements it for any OS that happens to have
the fcntl F_CLOSEM option.   It is below in its entirety.  I believe I
got the try: stuff correct, but since I'm new to Python, I'd appreciate
any comments.

I have another patch to implement my os.rclose(x,y) method, which would
improve the speed of popenX() for the OSes that don't have F_CLOSEM, by
doing the close() loop in C instead of Python,  but I don't know if it
would be as likely to be accepted as this probably would be.

Now, where do I send my proposed patch for consideration?

John.

--- ./Modules/fcntlmodule.c.orig	Thu Jun  3 08:47:26 2004
+++ ./Modules/fcntlmodule.c	Fri Nov 24 11:18:23 2006
@@ -545,6 +545,11 @@
         if (ins(d, "DN_MULTISHOT", (long)DN_MULTISHOT)) return -1;
 #endif

+/* For systems like AIX that have F_CLOSEM to close multiple fds */
+#ifdef F_CLOSEM
+        if (ins(d, "F_CLOSEM", (long)F_CLOSEM)) return -1;
+#endif
+
 #ifdef HAVE_STROPTS_H
 	/* Unix 98 guarantees that these are in stropts.h. */
 	INS(I_PUSH);
--- ./Lib/popen2.py.orig	Thu Feb 10 08:46:14 2005
+++ ./Lib/popen2.py	Fri Nov 24 11:37:15 2006
@@ -8,6 +8,7 @@

 import os
 import sys
+import fcntl

 __all__ = ["popen2", "popen3", "popen4"]

@@ -65,11 +66,18 @@
     def _run_child(self, cmd):
         if isinstance(cmd, basestring):
             cmd = ['/bin/sh', '-c', cmd]
-        for i in range(3, MAXFD):
-            try:
-                os.close(i)
-            except OSError:
-                pass
+	try:
+	    if fcntl.F_CLOSEM:
+		try:
+		    fcntl.fcntl(3, fcntl.F_CLOSEM, 0)
+		except OSError:
+		    pass
+	except AttributeError:
+	    for i in range(3, MAXFD):
+		try:
+		    os.close(i)
+		except OSError:
+		    pass
         try:
             os.execvp(cmd[0], cmd)
         finally: