Don't do double-serial Pylint checks

Pylint uses parallelism to improve performance, but high startup costs
means that this only makes sense if there are lots of files to be
processed. So, a while ago a change was made such that if there are
fewer than ten files to be analyzed then no parallelism is used.

Our Pylint wrapper also has a hack where it does one type of check in
serial mode, because that is the only time it is reliable. This
requires running Pylint twice, which is expensive.

If there are a small enough number of files to analyze then we will be
doing serial analysis anyway, so there is no need to do two separate
runs. In this test case:

  git cl presubmit -v --files tools\code_coverage\create_js_source_maps\test\create_js_source_maps_test.py

the cost of Pylint is dropped roughly in half, from six seconds to
three seconds, by eliminating one of the three-second runs.

Bug: 1309977
Change-Id: I2e5e96a86d1d76b127f481af7478d807c042b609
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3812436Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

presubmit_canned_checks.py

@@ -1071,6 +1071,7 @@ def GetPylint(input_api,
   env['PYTHONPATH'] = input_api.os_path.pathsep.join(extra_paths_list)
   env.pop('VPYTHON_CLEAR_PYTHONPATH', None)
   input_api.logging.debug('  with extra PYTHONPATH: %r', extra_paths_list)
+  files_per_job = 10
 
   def GetPylintCmd(flist, extra, parallel):
     # Windows needs help running python files so we explicitly specify
@@ -1103,9 +1104,10 @@ def GetPylint(input_api,
       # 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
+      jobs = min(input_api.cpu_count, 1 + len(flist) // files_per_job)
+      if jobs > 1:
+        args.append('--jobs=%s' % jobs)
+        description += ' on %d processes' % jobs
 
     kwargs['stdin'] = '\n'.join(args + flist)
     if input_api.sys.version_info.major != 2:
@@ -1119,9 +1121,11 @@ def GetPylint(input_api,
         python3=not python2)
 
   # pylint's cycle detection doesn't work in parallel, so spawn a second,
-  # single-threaded job for just that check.
+  # single-threaded job for just that check. However, only do this if there are
+  # actually enough files to process to justify parallelism in the first place.
   # Some PRESUBMITs explicitly mention cycle detection.
-  if not any('R0401' in a or 'cyclic-import' in a for a in extra_args):
+  if len(files) >= files_per_job and not any(
+      'R0401' in a or 'cyclic-import' in a for a in extra_args):
     tests = [
       GetPylintCmd(files, ["--disable=cyclic-import"], True),
       GetPylintCmd(files, ["--disable=all", "--enable=cyclic-import"], False),

tests/presubmit_unittest.py

@@ -2456,18 +2456,10 @@ the current line as well!
             [pylint, '--args-on-stdin'], env=env,
             cwd='CWD', stderr=subprocess.STDOUT, stdout=subprocess.PIPE,
             stdin=subprocess.PIPE),
-        mock.call(
-            [pylint, '--args-on-stdin'], env=env,
-            cwd='CWD', stderr=subprocess.STDOUT, stdout=subprocess.PIPE,
-            stdin=subprocess.PIPE),
     ])
     self.assertEqual(presubmit.sigint_handler.wait.mock_calls, [
-        mock.call(
-            process,
-            ('--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=1\nfile1.py' %
+                  ('--rcfile=%s\nfile1.py' %
                    pylintrc).encode('utf-8')),
     ])