D3962: worker: ability to disable worker for CPU heavy tasks

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The worker on Windows is implemented using a thread pool. If worker
  tasks are executing CPU bound code and holding onto the GIL, there
  will be non-substantial overhead in Python context switching between
  active threads. This can result in significant slowdowns of tasks.
  
  This commit teaches the code for determining whether to use a worker
  to take CPU heavy tasks into account. If work is CPU heavy, the
  thread based Windows worker implementation rejects it.
  
  By itself, this patch is backwards compatible and shouldn't be overly
  contentious. I concede the "cost" of "CPU heavy" work should probably
  be a number and worked into an algorithm somehow. But I'm not sure
  what the algorithm should be and a bool is the easiest to implement.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3962

AFFECTED FILES
  mercurial/worker.py
  tests/test-simple-update.t

CHANGE DETAILS

diff --git a/tests/test-simple-update.t b/tests/test-simple-update.t
--- a/tests/test-simple-update.t
+++ b/tests/test-simple-update.t
@@ -65,7 +65,7 @@
 
   $ cat <<EOF > forceworker.py
   > from mercurial import extensions, worker
-  > def nocost(orig, ui, costperop, nops):
+  > def nocost(orig, ui, costperop, nops, cpuheavy=False):
   >     return worker._numworkers(ui) > 1
   > def uisetup(ui):
   >     extensions.wrapfunction(worker, 'worthwhile', nocost)
diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -57,18 +57,27 @@
 
 if pycompat.isposix or pycompat.iswindows:
     _STARTUP_COST = 0.01
+    # The Windows worker is thread based. If tasks are CPU bound, threads
+    # in the presence of the GIL result in excessive context switching and
+    # this overhead can slow down execution.
+    _ALLOW_CPU_HEAVY = False
 else:
     _STARTUP_COST = 1e30
+    _ALLOW_CPU_HEAVY = True
 
-def worthwhile(ui, costperop, nops):
+def worthwhile(ui, costperop, nops, cpuheavy=False):
     '''try to determine whether the benefit of multiple processes can
     outweigh the cost of starting them'''
+
+    if cpuheavy and not _ALLOW_CPU_HEAVY:
+        return False
+
     linear = costperop * nops
     workers = _numworkers(ui)
     benefit = linear - (_STARTUP_COST * workers + linear / workers)
     return benefit >= 0.15
 
-def worker(ui, costperarg, func, staticargs, args):
+def worker(ui, costperarg, func, staticargs, args, cpuheavy=False):
     '''run a function, possibly in parallel in multiple worker
     processes.
 
@@ -82,9 +91,13 @@
 
     args - arguments to split into chunks, to pass to individual
     workers
+
+    cpuheavy - whether each individual item of work requires a lot of CPU.
+    CPU bound tasks may not use workers if e.g. the worker is implemented by
+    thread pools (which are subject to the GIL).
     '''
     enabled = ui.configbool('worker', 'enabled')
-    if enabled and worthwhile(ui, costperarg, len(args)):
+    if enabled and worthwhile(ui, costperarg, len(args), cpuheavy=cpuheavy):
         return _platformworker(ui, func, staticargs, args)
     return func(*staticargs + (args,))
 



To: indygreg, #hg-reviewers
Cc: mercurial-devel