Mitigate Python 3 multiprocessing bug on Windows

The multiprocessing module on Windows has a bug where if you ask for
more than 60 child processes then it will hang. This is related to the
MAXIMUM_WAIT_OBJECTS (64) limit of WaitForMultipleObjects. Other sources
have listed the multiprocessing limit as being 61, or have said that the
the maximum number of objects that can be waited on is actually 63, but
those details don't really matter.

The original fix for this class of issues was crrev.com/c/2785844. This
change extends those fixes to depot_tools, which was missed last year.
This change also updates how PyLint is called by further limiting the
number of jobs to the number of files being processed divided by 10.
This is because there is a significant cost to creating PyLint
subprocesses - each takes about 0.5 s on my test machine. So there needs
to be enough parallelism to justify this.

Patches for PyLint and a bug for cpython are planned.

This will stop PyLint from hanging during presubmits on many-core
machines. The command used to reproduce the hangs and validate the fix
was:

  git cl presubmit -v --force --files "chrome/test/mini_installer/*.py"

Prior to this change this command would use (on my many-core test
machine) 96 processes and would hang. How it uses just two processes
because there are only 16 files to analyze.

Output before:
  Pylint (16 files using ['--disable=cyclic-import'] on 96 cores)
Output after:
  Pylint (16 files using ['--disable=cyclic-import'] on 2 processes)

This is actually not quite true because the hang would prevent the
old message from being displayed.

Bug: 1190269, 1336854
Change-Id: Ie82baf91df4364a92eb664a00cf9daf167e0a548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3711282Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

presubmit_canned_checks.py

@@ -1093,8 +1093,12 @@ def GetPylint(input_api,
       args.extend(extra)
       description += ' using %s' % (extra,)
     if parallel:
-      args.append('--jobs=%s' % input_api.cpu_count)
-      description += ' on %d cores' % input_api.cpu_count
+      # Make sure we don't request more parallelism than is justified for the
+      # number of files we have to process. PyLint child-process startup time is
+      # significant.
+      jobs = min(input_api.cpu_count, 1 + len(flist) // 10)
+      args.append('--jobs=%s' % jobs)
+      description += ' on %d processes' % jobs
 
     kwargs['stdin'] = '\n'.join(args + flist)
     if input_api.sys.version_info.major != 2:

presubmit_support.py

@@ -169,6 +169,10 @@ class ThreadPool(object):
   def __init__(self, pool_size=None, timeout=None):
     self.timeout = timeout
     self._pool_size = pool_size or multiprocessing.cpu_count()
+    if sys.platform == 'win32':
+      # TODO(crbug.com/1190269) - we can't use more than 56 child processes on
+      # Windows or Python3 may hang.
+      self._pool_size = min(self._pool_size, 56)
     self._messages = []
     self._messages_lock = threading.Lock()
     self._tests = []
@@ -658,6 +662,10 @@ class InputApi(object):
     self.platform = sys.platform
 
     self.cpu_count = multiprocessing.cpu_count()
+    if self.is_windows:
+      # TODO(crbug.com/1190269) - we can't use more than 56 child processes on
+      # Windows or Python3 may hang.
+      self.cpu_count = min(self.cpu_count, 56)
 
     # The local path of the currently-being-processed presubmit script.
     self._current_presubmit_path = os.path.dirname(presubmit_path)

tests/presubmit_unittest.py

@@ -2467,7 +2467,7 @@ the current line as well!
             ('--rcfile=%s\n--disable=all\n--enable=cyclic-import\nfile1.py' %
              pylintrc).encode('utf-8')),
         mock.call(process,
-                  ('--rcfile=%s\n--disable=cyclic-import\n--jobs=2\nfile1.py' %
+                  ('--rcfile=%s\n--disable=cyclic-import\n--jobs=1\nfile1.py' %
                    pylintrc).encode('utf-8')),
     ])