Patchwork [4,of,6] plan9: prevent potential wait()

login
register
mail settings
Submitter jas@corpus-callosum.com
Date Aug. 15, 2013, 11:19 p.m.
Message ID <9A876D83-ECE0-43C5-8E95-2059685B5CF7@corpus-callosum.com>
Download mbox | patch
Permalink /patch/2179/
State Rejected
Headers show

Comments

jas@corpus-callosum.com - Aug. 15, 2013, 11:19 p.m.
On Aug 15, 2013, at 4:59 PM, Matt Mackall <mpm@selenic.com> wrote:

> On Tue, 2013-08-13 at 21:20 -0500, Jeff Sickel wrote:
>> A small number, less than 130 forked children, may be
>> beneficial on Plan 9 (see following test script).
> 
> These are concurrent children, yes? The worker code should start up
> children proportional to CPUs. How many CPUs do you have?



countcpus() returns 1 as we don't support os.sysconf('SC_NPROCESSORS_ONLN').


To test I've put some print statements in worker.py:

acme# hg diff mercurial/worker.py



acme# hg up -C 2.7-plan9
workers: 4
pids: [608898, 608897, 608896, 608895]
Exception in thread Thread-1:
Traceback (most recent call last):
  File "/sys/lib/python2.7/threading.py:808", in __bootstrap_inner
    self.run()
  File "/sys/lib/python2.7/threading.py:761", in run
    self.__target(*self.__args, **self.__kwargs)
  File "/sys/lib/python2.7/site-packages/mercurial/worker.py:110", in waitforworkers
    st = _exitstatus(os.wait()[1])
OSError: [Errno 6] No children

workers: 4
pids: [608903, 608902, 608901, 608900]
Exception in thread Thread-2:
Traceback (most recent call last):
  File "/sys/lib/python2.7/threading.py:808", in __bootstrap_inner
    self.run()
  File "/sys/lib/python2.7/threading.py:761", in run
    self.__target(*self.__args, **self.__kwargs)
  File "/sys/lib/python2.7/site-packages/mercurial/worker.py:110", in waitforworkers
    st = _exitstatus(os.wait()[1])
OSError: [Errno 6] No children

3748 files updated, 0 files merged, 818 files removed, 0 files unresolved



In this case the files updated, but during a clone it will rollback
and remove the pulled local repository.
Matt Mackall - Aug. 16, 2013, 3:08 p.m.
On Thu, 2013-08-15 at 18:19 -0500, Jeff Sickel wrote:
> On Aug 15, 2013, at 4:59 PM, Matt Mackall <mpm@selenic.com> wrote:
> 
> > On Tue, 2013-08-13 at 21:20 -0500, Jeff Sickel wrote:
> >> A small number, less than 130 forked children, may be
> >> beneficial on Plan 9 (see following test script).
> > 
> > These are concurrent children, yes? The worker code should start up
> > children proportional to CPUs. How many CPUs do you have?
> 
> 
> 
> countcpus() returns 1 as we don't support os.sysconf('SC_NPROCESSORS_ONLN').

Ok, you've found two bugs:

a) we're going multithreaded on single core machines (not a good idea)
b) your os.wait() is extra-busted

If we fix (a), then (b) will be less relevant.

Patch

diff -r 5d2d7f71b19a mercurial/worker.py
--- a/mercurial/worker.py	Mon Aug 12 17:48:27 2013 -0500
+++ b/mercurial/worker.py	Thu Aug 15 17:56:30 2013 -0500
@@ -48,11 +48,9 @@ 
 def worthwhile(ui, costperop, nops):
     '''try to determine whether the benefit of multiple processes can
     outweigh the cost of starting them'''
-    # this trivial calculation does not benefit plan 9
-    if sys.platform == 'plan9':
-        return 0
     linear = costperop * nops
     workers = _numworkers(ui)
+    print "workers: %d" % (workers)
     benefit = linear - (_startupcost * workers + linear / workers)
     return benefit >= 0.15
 
@@ -96,6 +94,7 @@ 
                 # on lock.py's pid checks to avoid release callbacks
         pids.append(pid)
     pids.reverse()
+    print "pids: %s" % (pids)
     os.close(wfd)
     fp = os.fdopen(rfd, 'rb', 0)
     def killworkers():