Bug 585774: Revert pymake to an earlier version to fix a massive build perf loss.

build/pymake/.hg_archival.txt

@@ -1,5 +1,2 @@
 repo: f5ab154deef2ffa97f1b2139589ae4a1962090a4
-node: e50be3234e397322410b29a9e226365e23cb322e
+node: 7ae0b4af32617677698f9de3ab76bcb154bbf085
-branch: default
-latesttag: null
-latesttagdistance: 255

build/pymake/make.py

@@ -10,10 +10,8 @@ import sys, os
 import pymake.command, pymake.process
 
 import gc
+gc.disable()
 
-if __name__ == '__main__':
+pymake.command.main(sys.argv[1:], os.environ, os.getcwd(), cb=sys.exit)
-  gc.disable()
+pymake.process.ParallelContext.spin()
-
+assert False, "Not reached"
-  pymake.command.main(sys.argv[1:], os.environ, os.getcwd(), cb=sys.exit)
-  pymake.process.ParallelContext.spin()
-  assert False, "Not reached"

build/pymake/pymake/builtins.py

@@ -1,68 +1,10 @@
-# Basic commands implemented in Python
+"""
-import sys, os, shutil, time
+Implicit variables; perhaps in the future this will also include some implicit
-from getopt import getopt, GetoptError
+rules, at least match-anything cancellation rules.
+"""
 
-from process import PythonException
+variables = {
-
+    'RM': 'rm -f',
-__all__ = ["rm", "sleep", "touch"]
+    '.LIBPATTERNS': 'lib%.so lib%.a',
-
+    '.PYMAKE': '1',
-def rm(args):
+    }
-  """
-  Emulate most of the behavior of rm(1).
-  Only supports the -r (--recursive) and -f (--force) arguments.
-  """
-  try:
-    opts, args = getopt(args, "rRf", ["force", "recursive"])
-  except GetoptError, e:
-    raise PythonException, ("rm: %s" % e, 1)
-  force = False
-  recursive = False
-  for o, a in opts:
-    if o in ('-f', '--force'):
-      force = True
-    elif o in ('-r', '-R', '--recursive'):
-      recursive = True
-  for f in args:
-    if os.path.isdir(f):
-      if not recursive:
-        raise PythonException, ("rm: cannot remove '%s': Is a directory" % f, 1)
-      else:
-        shutil.rmtree(f, force)
-    elif os.path.exists(f):
-      try:
-        os.unlink(f)
-      except:
-        if not force:
-          raise PythonException, ("rm: failed to remove '%s': %s" % (f, sys.exc_info()[0]), 1)
-    elif not force:
-      raise PythonException, ("rm: cannot remove '%s': No such file or directory" % f, 1)
-
-def sleep(args):
-    """
-    Emulate the behavior of sleep(1).
-    """
-    total = 0
-    values = {'s': 1, 'm': 60, 'h': 3600, 'd': 86400}
-    for a in args:
-        multiplier = 1
-        for k, v in values.iteritems():
-            if a.endswith(k):
-                a = a[:-1]
-                multiplier = v
-                break
-        try:
-            f = float(a)
-            total += f * multiplier
-        except ValueError:
-            raise PythonException, ("sleep: invalid time interval '%s'" % a, 1)
-    time.sleep(total)
-
-def touch(args):
-    """
-    Emulate the behavior of touch(1).
-    """
-    for f in args:
-        if os.path.exists(f):
-            os.utime(f, None)
-        else:
-            open(f, 'w').close()

build/pymake/pymake/command.py

@@ -106,8 +106,7 @@ class _MakeContext(object):
                                           makeoverrides=self.overrides,
                                           workdir=self.workdir,
                                           context=self.context, env=self.env, makelevel=self.makelevel,
-                                          targets=self.targets, keepgoing=self.options.keepgoing,
+                                          targets=self.targets, keepgoing=self.options.keepgoing)
-                                          silent=self.options.silent)
 
             self.restarts += 1
 
@@ -187,8 +186,6 @@ def main(args, env, cwd, cb):
                       dest="printdir")
         op.add_option('--no-print-directory', action="store_false",
                       dest="printdir", default=True)
-        op.add_option('-s', '--silent', action="store_true",
-                      dest="silent", default=False)
 
         options, arguments1 = op.parse_args(parsemakeflags(env))
         options, arguments2 = op.parse_args(args, values=options)
@@ -211,10 +208,6 @@ def main(args, env, cwd, cb):
         if options.printdir:
             shortflags.append('w')
 
-        if options.silent:
-            shortflags.append('s')
-            options.printdir = False
-
         loglevel = logging.WARNING
         if options.verbose:
             loglevel = logging.DEBUG

build/pymake/pymake/data.py

@@ -3,7 +3,7 @@ A representation of makefile data structures.
 """
 
 import logging, re, os, sys
-import parserdata, parser, functions, process, util, implicit
+import parserdata, parser, functions, process, util, builtins
 from cStringIO import StringIO
 
 _log = logging.getLogger('pymake.data')
@@ -1142,18 +1142,17 @@ def splitcommand(command):
 
 def findmodifiers(command):
     """
-    Find any of +-@% prefixed on the command.
+    Find any of +-@ prefixed on the command.
-    @returns (command, isHidden, isRecursive, ignoreErrors, isNative)
+    @returns (command, isHidden, isRecursive, ignoreErrors)
     """
 
     isHidden = False
     isRecursive = False
     ignoreErrors = False
-    isNative = False
 
-    realcommand = command.lstrip(' \t\n@+-%')
+    realcommand = command.lstrip(' \t\n@+-')
     modset = set(command[:-len(realcommand)])
-    return realcommand, '@' in modset, '+' in modset, '-' in modset, '%' in modset
+    return realcommand, '@' in modset, '+' in modset, '-' in modset
 
 class _CommandWrapper(object):
     def __init__(self, cline, ignoreErrors, loc, context, **kwargs):
@@ -1174,32 +1173,6 @@ class _CommandWrapper(object):
         self.usercb = cb
         process.call(self.cline, loc=self.loc, cb=self._cb, context=self.context, **self.kwargs)
 
-class _NativeWrapper(_CommandWrapper):
-    def __init__(self, cline, ignoreErrors, loc, context,
-                 pycommandpath, **kwargs):
-        _CommandWrapper.__init__(self, cline, ignoreErrors, loc, context,
-                                 **kwargs)
-        # get the module and method to call
-        parts, badchar = process.clinetoargv(cline)
-        if parts is None:
-            raise DataError("native command '%s': shell metacharacter '%s' in command line" % (cline, badchar), self.loc)
-        if len(parts) < 2:
-            raise DataError("native command '%s': no method name specified" % cline, self.loc)
-        if pycommandpath:
-            self.pycommandpath = re.split('[%s\s]+' % os.pathsep,
-                                          pycommandpath)
-        else:
-            self.pycommandpath = None
-        self.module = parts[0]
-        self.method = parts[1]
-        self.cline_list = parts[2:]
-
-    def __call__(self, cb):
-        self.usercb = cb
-        process.call_native(self.module, self.method, self.cline_list,
-                            loc=self.loc, cb=self._cb, context=self.context,
-                            pycommandpath=self.pycommandpath, **self.kwargs)
-
 def getcommandsforrule(rule, target, makefile, prerequisites, stem):
     v = Variables(parent=target.variables)
     setautomaticvariables(v, makefile, target, prerequisites)
@@ -1211,22 +1184,13 @@ def getcommandsforrule(rule, target, makefile, prerequisites, stem):
     for c in rule.commands:
         cstring = c.resolvestr(makefile, v)
         for cline in splitcommand(cstring):
-            cline, isHidden, isRecursive, ignoreErrors, isNative = findmodifiers(cline)
+            cline, isHidden, isRecursive, ignoreErrors = findmodifiers(cline)
-            if isHidden or makefile.silent:
+            if isHidden:
                 echo = None
             else:
                 echo = "%s$ %s" % (c.loc, cline)
-            if not isNative:
+            yield _CommandWrapper(cline, ignoreErrors=ignoreErrors, env=env, cwd=makefile.workdir, loc=c.loc, context=makefile.context,
-                yield _CommandWrapper(cline, ignoreErrors=ignoreErrors, env=env, cwd=makefile.workdir, loc=c.loc, context=makefile.context,
+                                 echo=echo)
-                                      echo=echo)
-            else:
-                f, s, e = v.get("PYCOMMANDPATH", True)
-                if e:
-                    e = e.resolvestr(makefile, v, ["PYCOMMANDPATH"])
-                yield _NativeWrapper(cline, ignoreErrors=ignoreErrors,
-                                     env=env, cwd=makefile.workdir,
-                                     loc=c.loc, context=makefile.context,
-                                     echo=echo, pycommandpath=e)
 
 class Rule(object):
     """
@@ -1371,8 +1335,7 @@ class Makefile(object):
 
     def __init__(self, workdir=None, env=None, restarts=0, make=None,
                  makeflags='', makeoverrides='',
-                 makelevel=0, context=None, targets=(), keepgoing=False,
+                 makelevel=0, context=None, targets=(), keepgoing=False):
-                 silent=False):
         self.defaulttarget = None
 
         if env is None:
@@ -1386,7 +1349,6 @@ class Makefile(object):
         self.exportedvars = {}
         self._targets = {}
         self.keepgoing = keepgoing
-        self.silent = silent
         self._patternvariables = [] # of (pattern, variables)
         self.implicitrules = []
         self.parsingfinished = False
@@ -1406,8 +1368,6 @@ class Makefile(object):
         self.variables.set('MAKE_RESTARTS', Variables.FLAVOR_SIMPLE,
                            Variables.SOURCE_AUTOMATIC, restarts > 0 and str(restarts) or '')
 
-        self.variables.set('.PYMAKE', Variables.FLAVOR_SIMPLE,
-                           Variables.SOURCE_MAKEFILE, "1")
         if make is not None:
             self.variables.set('MAKE', Variables.FLAVOR_SIMPLE,
                                Variables.SOURCE_MAKEFILE, make)
@@ -1432,7 +1392,7 @@ class Makefile(object):
         self.variables.set('MAKECMDGOALS', Variables.FLAVOR_SIMPLE,
                            Variables.SOURCE_AUTOMATIC, ' '.join(targets))
 
-        for vname, val in implicit.variables.iteritems():
+        for vname, val in builtins.variables.iteritems():
             self.variables.set(vname,
                                Variables.FLAVOR_SIMPLE,
                                Variables.SOURCE_IMPLICIT, val)

build/pymake/pymake/implicit.py

@@ -1,13 +0,0 @@
-"""
-Implicit variables; perhaps in the future this will also include some implicit
-rules, at least match-anything cancellation rules.
-"""
-
-variables = {
-    'RM': '%pymake.builtins rm -f',
-    'SLEEP': '%pymake.builtins sleep',
-    'TOUCH': '%pymake.builtins touch',
-    '.LIBPATTERNS': 'lib%.so lib%.a',
-    '.PYMAKE': '1',
-    }
-

build/pymake/pymake/process.py

@@ -3,9 +3,7 @@ Skipping shell invocations is good, when possible. This wrapper around subproces
 parsing command lines into argv and making sure that no shell magic is being used.
 """
 
-#TODO: ship pyprocessing?
+import subprocess, shlex, re, logging, sys, traceback, os
-from multiprocessing import Pool, Condition
-import subprocess, shlex, re, logging, sys, traceback, os, imp
 import command, util
 if sys.platform=='win32':
     import win32process
@@ -81,11 +79,6 @@ def call(cline, env, cwd, loc, cb, context, echo):
 
     context.call(argv, executable=executable, shell=False, env=env, cwd=cwd, cb=cb, echo=echo)
 
-def call_native(module, method, argv, env, cwd, loc, cb, context, echo,
-                pycommandpath=None):
-    context.call_native(module, method, argv, env=env, cwd=cwd, cb=cb,
-                        echo=echo, pycommandpath=pycommandpath)
-
 def statustoresult(status):
     """
     Convert the status returned from waitpid into a prettier numeric result.
@@ -96,127 +89,17 @@ def statustoresult(status):
 
     return status >>8
 
-class Job(object):
-    """
-    A single job to be executed on the process pool.
-    """
-    done = False # set to true when the job completes
-
-    def __init__(self):
-        self.exitcode = -127
-
-    def notify(self, condition, result):
-        condition.acquire()
-        self.done = True
-        self.exitcode = result
-        condition.notify()
-        condition.release()
-
-    def get_callback(self, condition):
-        return lambda result: self.notify(condition, result)
-
-class PopenJob(Job):
-    """
-    A job that executes a command using subprocess.Popen.
-    """
-    def __init__(self, argv, executable, shell, env, cwd):
-        Job.__init__(self)
-        self.argv = argv
-        self.executable = executable
-        self.shell = shell
-        self.env = env
-        self.cwd = cwd
-
-    def run(self):
-        try:
-            p = subprocess.Popen(self.argv, executable=self.executable, shell=self.shell, env=self.env, cwd=self.cwd)
-            return p.wait()
-        except OSError, e:
-            print >>sys.stderr, e
-            return -127
-
-class PythonException(Exception):
-    def __init__(self, message, exitcode):
-        Exception.__init__(self)
-        self.message = message
-        self.exitcode = exitcode
-
-    def __str__(self):
-        return self.message
-
-def load_module_recursive(module, path):
-    """
-    Emulate the behavior of __import__, but allow
-    passing a custom path to search for modules.
-    """
-    bits = module.split('.')
-    for i, bit in enumerate(bits):
-        dotname = '.'.join(bits[:i+1])
-        try:
-          f, path, desc = imp.find_module(bit, path)
-          m = imp.load_module(dotname, f, path, desc)
-          if f is None:
-              path = m.__path__
-        except ImportError:
-            return
-
-class PythonJob(Job):
-    """
-    A job that calls a Python method.
-    """
-    def __init__(self, module, method, argv, env, cwd, pycommandpath=None):
-        self.module = module
-        self.method = method
-        self.argv = argv
-        self.env = env
-        self.cwd = cwd
-        self.pycommandpath = pycommandpath or []
-
-    def run(self):
-        oldenv = os.environ
-        try:
-            os.chdir(self.cwd)
-            os.environ = self.env
-            if self.module not in sys.modules:
-                load_module_recursive(self.module,
-                                      sys.path + self.pycommandpath)
-            if self.module not in sys.modules:
-                print >>sys.stderr, "No module named '%s'" % self.module
-                return -127                
-            m = sys.modules[self.module]
-            if self.method not in m.__dict__:
-                print >>sys.stderr, "No method named '%s' in module %s" % (method, module)
-                return -127
-            m.__dict__[self.method](self.argv)
-        except PythonException, e:
-            print >>sys.stderr, e
-            return e.exitcode
-        except:
-            print >>sys.stderr, sys.exc_info()[1]
-            return -127
-        finally:
-            os.environ = oldenv
-        return 0
-
-def job_runner(job):
-    """
-    Run a job. Called in a Process pool.
-    """
-    return job.run()
-
 class ParallelContext(object):
     """
     Manages the parallel execution of processes.
     """
 
     _allcontexts = set()
-    _condition = Condition()
 
     def __init__(self, jcount):
         self.jcount = jcount
         self.exit = False
 
-        self.pool = Pool(processes=jcount)
         self.pending = [] # list of (cb, args, kwargs)
         self.running = [] # list of (subprocess, cb)
 
@@ -224,8 +107,6 @@ class ParallelContext(object):
 
     def finish(self):
         assert len(self.pending) == 0 and len(self.running) == 0, "pending: %i running: %i" % (len(self.pending), len(self.running))
-        self.pool.close()
-        self.pool.join()
         self._allcontexts.remove(self)
 
     def run(self):
@@ -238,19 +119,16 @@ class ParallelContext(object):
         self.pending.append((cb, args, kwargs))
 
     def _docall(self, argv, executable, shell, env, cwd, cb, echo):
-        if echo is not None:
+            if echo is not None:
-            print echo
+                print echo
-        job = PopenJob(argv, executable=executable, shell=shell, env=env, cwd=cwd)
+            try:
-        self.pool.apply_async(job_runner, args=(job,), callback=job.get_callback(ParallelContext._condition))
+                p = subprocess.Popen(argv, executable=executable, shell=shell, env=env, cwd=cwd)
-        self.running.append((job, cb))
+            except OSError, e:
+                print >>sys.stderr, e
+                cb(-127)
+                return
 
-    def _docallnative(self, module, method, argv, env, cwd, cb, echo,
+            self.running.append((p, cb))
-                      pycommandpath=None):
-        if echo is not None:
-            print echo
-        job = PythonJob(module, method, argv, env, cwd, pycommandpath)
-        self.pool.apply_async(job_runner, args=(job,), callback=job.get_callback(ParallelContext._condition))
-        self.running.append((job, cb))
 
     def call(self, argv, shell, env, cwd, cb, echo, executable=None):
         """
@@ -259,42 +137,23 @@ class ParallelContext(object):
 
         self.defer(self._docall, argv, executable, shell, env, cwd, cb, echo)
 
-    def call_native(self, module, method, argv, env, cwd, cb,
+    if sys.platform == 'win32':
-                    echo, pycommandpath=None):
+        @staticmethod
-        """
+        def _waitany():
-        Asynchronously call the native function
+            return win32process.WaitForAnyProcess([p for c in ParallelContext._allcontexts for p, cb in c.running])
-        """
 
-        self.defer(self._docallnative, module, method, argv, env, cwd, cb,
+        @staticmethod
-                   echo, pycommandpath)
+        def _comparepid(pid, process):
+            return pid == process
 
-    @staticmethod
+    else:
-    def _waitany():
+        @staticmethod
-        def _checkdone():
+        def _waitany():
-            jobs = []
+            return os.waitpid(-1, 0)
-            for c in ParallelContext._allcontexts:
-                for i in xrange(0, len(c.running)):
-                    if c.running[i][0].done:
-                        jobs.append(c.running[i])
-                for j in jobs:
-                    if j in c.running:
-                        c.running.remove(j)
-            return jobs
 
-        # We must acquire the lock, and then check to see if any jobs have
+        @staticmethod
-        # finished.  If we don't check after acquiring the lock it's possible
+        def _comparepid(pid, process):
-        # that all outstanding jobs will have completed before we wait and we'll
+            return pid == process.pid
-        # wait for notifications that have already occurred.
-        ParallelContext._condition.acquire()
-        jobs = _checkdone()
-
-        if jobs == []:
-            ParallelContext._condition.wait()
-            jobs = _checkdone()
-
-        ParallelContext._condition.release()
-
-        return jobs
 
     @staticmethod
     def spin():
@@ -307,10 +166,39 @@ class ParallelContext(object):
             for c in clist:
                 c.run()
 
+            # In python 2.4, subprocess instances wait on child processes under the hood when they are created... this
+            # unfortunate behavior means that before using os.waitpid, we need to check the status using .poll()
+            # see http://bytes.com/groups/python/675403-os-wait-losing-child
+            found = False
+            for c in clist:
+                for i in xrange(0, len(c.running)):
+                    p, cb = c.running[i]
+                    result = p.poll()
+                    if result != None:
+                        del c.running[i]
+                        cb(result)
+                        found = True
+                        break
+
+                if found: break
+            if found: continue
+
             dowait = util.any((len(c.running) for c in ParallelContext._allcontexts))
+
             if dowait:
-                for job, cb in ParallelContext._waitany():
+                pid, status = ParallelContext._waitany()
-                    cb(job.exitcode)
+                result = statustoresult(status)
+
+                for c in ParallelContext._allcontexts:
+                    for i in xrange(0, len(c.running)):
+                        p, cb = c.running[i]
+                        if ParallelContext._comparepid(pid, p):
+                            del c.running[i]
+                            cb(result)
+                            found = True
+                            break
+
+                    if found: break
             else:
                 assert any(len(c.pending) for c in ParallelContext._allcontexts)
 

build/pymake/tests/native-environment.mk

@@ -1,10 +0,0 @@
-#T gmake skip
-export EXPECTED := some data
-
-CMD = %pycmd writeenvtofile
-PYCOMMANDPATH = $(TESTPATH)
-
-all:
-	$(CMD) results EXPECTED
-	test "$$(cat results)" = "$(EXPECTED)"
-	@echo TEST-PASS

build/pymake/tests/native-pycommandpath-sep.mk

@@ -1,21 +0,0 @@
-#T gmake skip
-EXPECTED := some data
-
-# verify that we can load native command modules from
-# multiple directories in PYCOMMANDPATH separated by the native
-# path separator
-ifdef __WIN32__
-PS:=;
-else
-PS:=:
-endif
-CMD = %pycmd writetofile
-CMD2 = %pymod writetofile
-PYCOMMANDPATH = $(TESTPATH)$(PS)$(TESTPATH)/subdir
-
-all:
-	$(CMD) results $(EXPECTED)
-	test "$$(cat results)" = "$(EXPECTED)"
-	$(CMD2) results2 $(EXPECTED)
-	test "$$(cat results2)" = "$(EXPECTED)"
-	@echo TEST-PASS

build/pymake/tests/native-pycommandpath.mk

@@ -1,15 +0,0 @@
-#T gmake skip
-EXPECTED := some data
-
-# verify that we can load native command modules from
-# multiple space-separated directories in PYCOMMANDPATH
-CMD = %pycmd writetofile
-CMD2 = %pymod writetofile
-PYCOMMANDPATH = $(TESTPATH) $(TESTPATH)/subdir
-
-all:
-	$(CMD) results $(EXPECTED)
-	test "$$(cat results)" = "$(EXPECTED)"
-	$(CMD2) results2 $(EXPECTED)
-	test "$$(cat results2)" = "$(EXPECTED)"
-	@echo TEST-PASS

build/pymake/tests/native-simple.mk

@@ -1,10 +0,0 @@
-ifndef TOUCH
-TOUCH = touch
-endif
-
-all: testfile
-	test -f testfile
-	@echo TEST-PASS
-
-testfile:
-	$(TOUCH) $@

build/pymake/tests/parallel-native.mk

@@ -1,21 +0,0 @@
-#T commandline: ['-j2']
-
-# ensure that calling python commands doesn't block other targets
-ifndef SLEEP
-SLEEP := sleep
-endif
-
-PRINTF = printf "$@:0:" >>results
-EXPECTED = target2:0:target1:0:
-
-all:: target1 target2
-	cat results
-	test "$$(cat results)" = "$(EXPECTED)"
-	@echo TEST-PASS
-
-target1:
-	$(SLEEP) 0.1
-	$(PRINTF)
-
-target2:
-	$(PRINTF)

build/pymake/tests/pycmd.py

@@ -1,9 +0,0 @@
-import os
-
-def writetofile(args):
-  with open(args[0], 'w') as f:
-    f.write(' '.join(args[1:]))
-
-def writeenvtofile(args):
-  with open(args[0], 'w') as f:
-    f.write(os.environ[args[1]])

build/pymake/tests/rm-fail.mk

@@ -1,7 +0,0 @@
-#T returncode: 2
-all:
-	mkdir newdir
-	test -d newdir
-	touch newdir/newfile
-	$(RM) newdir
-	@echo TEST-PASS

build/pymake/tests/rm.mk

@@ -1,13 +0,0 @@
-all:
-# $(RM) defaults to -f
-	$(RM) nosuchfile
-	touch newfile
-	test -f newfile
-	$(RM) newfile
-	test ! -f newfile
-	mkdir newdir
-	test -d newdir
-	touch newdir/newfile
-	$(RM) -r newdir
-	test ! -d newdir
-	@echo TEST-PASS

build/pymake/tests/runtests.py

@@ -1,4 +1,3 @@
-#!/usr/bin/env python
 """
 Run the test(s) listed on the command line. If a directory is listed, the script will recursively
 walk the directory for files named .mk and run each.

build/pymake/tests/subdir/pymod.py

@@ -1,3 +0,0 @@
-def writetofile(args):
-  with open(args[0], 'w') as f:
-    f.write(' '.join(args[1:]))