Request for inclusion in 2.5.2 (5-for-1)
Issue http://bugs.python.org/issue1663329 details an annoyance in the subprocess module that has affected several users, including me. Essentially, closing hundreds of thousands of file descriptors by round-tripping through the python exception machinery is very slow, taking hundreds of milliseconds and at times many seconds. The proposed fix is to write this loop in c. The c function is but a handful of lines long. I purposefully kept the implementation trivial so that it will work on all unix variants (there is another issue that contains a super-duper optimization for AIX, and other possibilities exist for Solaris, but the simple fix yields a ten-fold speedup everywhere but windows, so I didn't think that it was worth the complexity). Though technically relating only to performance, I consider this a bug-fix candidate as mysterious multi-second delays when launching a subprocess end up making the functionality of close_fds unusable on some platform configurations (namely, those with high MAX_FD set). It would be great to see this is 2.5.2. Understanding that issue evaluation takes significant effort, I've done some evaluation/triage on other open tickets: ---------------------------------------- See issues for detailed comments. http://bugs.python.org/issue1516330: No clear problem, invalid patch. Recommend rejection. http://bugs.python.org/issue1516327: No clear problem, no patch. Recommend closing. http://bugs.python.org/issue1705170: reproduced. Conjecture as to why it is occurring, but I don't know the guts well enough to propose a decent fix. http://bugs.python.org/issue1773632: tested patch. Recommend accepting unless there are things I don't know about this mysterious _xmlrpclib extension (which there doubtlessly are) http://bugs.python.org/issue738948: Rather old PEP that has gathered no comments. Calling it a "PEP" is generous--it is really just a link to an academic paper with a note about how this might be integrated into Stackless. Thanks, -Mike
Mike Klaas wrote:
http://bugs.python.org/issue1705170: reproduced. Conjecture as to why it is occurring, but I don't know the guts well enough to propose a decent fix.
I've fixed this on the trunk (I'm afraid I have no opinion on the patch you're interested in though. Neal - where does the 2.5 branch stand at the moment? This would be a simple fix to slip into 2.5.2 if there's still time. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Mike, thanks for reviewing the bugs and for creating a patch. Good work! On Nov 2, 2007 2:23 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Mike Klaas wrote:
http://bugs.python.org/issue1705170: reproduced. Conjecture as to why it is occurring, but I don't know the guts well enough to propose a decent fix.
I've fixed this on the trunk (I'm afraid I have no opinion on the patch you're interested in though.
Neal - where does the 2.5 branch stand at the moment? This would be a simple fix to slip into 2.5.2 if there's still time.
I'm not sure if this was before my last status mail. We are hoping to get a release candidate out in a week or two. I agree with Guido that this isn't a bugfix and therefore shouldn't be backported to 2.5. n
Neal Norwitz wrote:
On Nov 2, 2007 2:23 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Neal - where does the 2.5 branch stand at the moment? This would be a simple fix to slip into 2.5.2 if there's still time.
I'm not sure if this was before my last status mail. We are hoping to get a release candidate out in a week or two.
It was more than a week but less than two weeks after your last status message, but you hadn't said anything yet about freezing the branch to cut a release candidate - hence my question :)
I agree with Guido that this isn't a bugfix and therefore shouldn't be backported to 2.5.
I was actually referring to the contextmanager bug with that comment - I was just a little vague with my pronouns. All sorted now though. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
On 10/31/07, Mike Klaas <mike.klaas@gmail.com> wrote:
Issue http://bugs.python.org/issue1663329 details an annoyance in the subprocess module that has affected several users, including me. Essentially, closing hundreds of thousands of file descriptors by round-tripping through the python exception machinery is very slow, taking hundreds of milliseconds and at times many seconds. The proposed fix is to write this loop in c. The c function is but a handful of lines long. I purposefully kept the implementation trivial so that it will work on all unix variants (there is another issue that contains a super-duper optimization for AIX, and other possibilities exist for Solaris, but the simple fix yields a ten-fold speedup everywhere but windows, so I didn't think that it was worth the complexity).
Though technically relating only to performance, I consider this a bug-fix candidate as mysterious multi-second delays when launching a subprocess end up making the functionality of close_fds unusable on some platform configurations (namely, those with high MAX_FD set).
It would be great to see this is 2.5.2. Understanding that issue evaluation takes significant effort, I've done some evaluation/triage on other open tickets:
Thanks for doing these! Since people are already jumping on those bugs but nobody has voiced an opinion on your own patch, let me say that I think it's a good patch, and I want it in 2.6, but I'm reluctant to add it to 2.5.2 as it goes well beyond a bugfix (adding a new C API and all that). -- --Guido van Rossum (home page: http://www.python.org/~guido/)
On 2-Nov-07, at 6:57 AM, Guido van Rossum wrote:
Since people are already jumping on those bugs but nobody has voiced an opinion on your own patch, let me say that I think it's a good patch, and I want it in 2.6, but I'm reluctant to add it to 2.5.2 as it goes well beyond a bugfix (adding a new C API and all that).
Thanks for looking at it! Is there a better way of exposing some c-helper code for a stdlib module written in python? It seems that the canonical pattern is to write a separate extension module called _<modulename> and import the functionality from there, but that seemed like a significantly more invasive patch. Might it help to tack on the helper function in posix only, deleting it from the os namespace? Thanks again, -Mike
On 11/2/07, Mike Klaas <mike.klaas@gmail.com> wrote:
On 2-Nov-07, at 6:57 AM, Guido van Rossum wrote:
Since people are already jumping on those bugs but nobody has voiced an opinion on your own patch, let me say that I think it's a good patch, and I want it in 2.6, but I'm reluctant to add it to 2.5.2 as it goes well beyond a bugfix (adding a new C API and all that).
Thanks for looking at it!
Is there a better way of exposing some c-helper code for a stdlib module written in python? It seems that the canonical pattern is to write a separate extension module called _<modulename> and import the functionality from there, but that seemed like a significantly more invasive patch.
No, what you did was the right thing. It just doesn't feel like a bugfix to me.
Might it help to tack on the helper function in posix only, deleting it from the os namespace?
No. Why are yo so insistent on having this in 2.5.2? You can't force folks who use your code to upgrade (e.g. OSX Leopard was just shipped with 2.5.1). -- --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (4)
-
Guido van Rossum -
Mike Klaas -
Neal Norwitz -
Nick Coghlan