Patchwork D3962: worker: ability to disable thread unsafe tasks

login
register
mail settings
Submitter phabricator
Date July 18, 2018, 4:55 p.m.
Message ID <82e11101daa46821d934054efd1c4db4@localhost.localdomain>
Download mbox | patch
Permalink /patch/32902/
State Not Applicable
Headers show

Comments

phabricator - July 18, 2018, 4:55 p.m.
indygreg updated this revision to Diff 9623.
indygreg edited the summary of this revision.
indygreg retitled this revision from "worker: ability to disable worker for CPU heavy tasks" to "worker: ability to disable thread unsafe tasks".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3962?vs=9620&id=9623

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

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

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - July 19, 2018, 12:06 p.m.
>  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.
> +    _DISALLOW_THREAD_UNSAFE = True

Fixed this as `_DISALLOW_THREAD_UNSAFE = pycompat.iswindows`, and queued.
Thanks.
phabricator - July 19, 2018, 12:15 p.m.
yuja added a comment.


  >   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.
  >  +    _DISALLOW_THREAD_UNSAFE = True
  
  Fixed this as `_DISALLOW_THREAD_UNSAFE = pycompat.iswindows`, and queued.
  Thanks.

REPOSITORY
  rHG Mercurial

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

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

Patch

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, threadsafe=True):
   >     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.
+    _DISALLOW_THREAD_UNSAFE = True
 else:
     _STARTUP_COST = 1e30
+    _DISALLOW_THREAD_UNSAFE = False
 
-def worthwhile(ui, costperop, nops):
+def worthwhile(ui, costperop, nops, threadsafe=True):
     '''try to determine whether the benefit of multiple processes can
     outweigh the cost of starting them'''
+
+    if not threadsafe and _DISALLOW_THREAD_UNSAFE:
+        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, threadsafe=True):
     '''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
+
+    threadsafe - whether work items are thread safe and can be executed using
+    a thread-based worker. Should be disabled for CPU heavy tasks that don't
+    release the GIL.
     '''
     enabled = ui.configbool('worker', 'enabled')
-    if enabled and worthwhile(ui, costperarg, len(args)):
+    if enabled and worthwhile(ui, costperarg, len(args), threadsafe=threadsafe):
         return _platformworker(ui, func, staticargs, args)
     return func(*staticargs + (args,))