Patchwork D8270: run-tests: restrict Rust thread pool to 3 threads during tests

login
register
mail settings
Submitter phabricator
Date March 10, 2020, 3:28 p.m.
Message ID <differential-rev-PHID-DREV-o3b4lhotkit5dxe3ag5j-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45662/
State Superseded
Headers show

Comments

phabricator - March 10, 2020, 3:28 p.m.
Alphare created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

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

AFFECTED FILES
  tests/run-tests.py

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-devel
phabricator - March 10, 2020, 4:33 p.m.
pulkit added inline comments.

INLINE COMMENTS

> run-tests.py:2998
> +        # multi-threading bugs while keeping the thrashing reasonable.
> +        os.environ.setdefault("RAYON_NUM_THREADS", "3")
> +

Should we also take value of `-j` into account too? If `-j 1` is used, then we should not do multi-threading here.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8270/new/

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

To: Alphare, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 10, 2020, 8:34 p.m.
durin42 added inline comments.
durin42 accepted this revision as: durin42.

INLINE COMMENTS

> pulkit wrote in run-tests.py:2998
> Should we also take value of `-j` into account too? If `-j 1` is used, then we should not do multi-threading here.

I don't think so, since part of the goal is to uncover threading bugs.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8270/new/

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

To: Alphare, #hg-reviewers, durin42
Cc: durin42, pulkit, mercurial-devel
phabricator - March 11, 2020, 12:22 a.m.
marmoute added inline comments.

INLINE COMMENTS

> pulkit wrote in run-tests.py:2998
> Should we also take value of `-j` into account too? If `-j 1` is used, then we should not do multi-threading here.

I don't think we should take -j in account. Even if we run test one at a time, it is important to test the multithreading code path

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8270/new/

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

To: Alphare, #hg-reviewers, durin42
Cc: marmoute, durin42, pulkit, mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -2991,6 +2991,12 @@ 
             # we do the randomness ourself to know what seed is used
             os.environ['PYTHONHASHSEED'] = str(random.getrandbits(32))
 
+        # Rayon (Rust crate for multi-threading) will use all logical CPU cores
+        # by default, causing thrashing on high-cpu-count systems.
+        # Setting its limit to 3 during tests should still let us uncover
+        # multi-threading bugs while keeping the thrashing reasonable.
+        os.environ.setdefault("RAYON_NUM_THREADS", "3")
+
         if self.options.tmpdir:
             self.options.keep_tmpdir = True
             tmpdir = _bytespath(self.options.tmpdir)