Run unit tests in parallel #12926

sipa · 2018-04-09T20:35:12Z

This runs the unit tests (src/test/test_bitcoin) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32).

This uses an approach by @theuni that relies on make as the mechanism for distributing tests over processes (through -j). For every test .cpp file, we search for BOOST_FIXTURE_TEST_SUITE or BOOST_AUTO_TEST_SUITE, and then invoke the test binary for just that suite (using -t). The (verbose) output is stored in a temporary file, and only shown in the case of failure.

Some makefile reshuffling is necessary to avoid trying to run tests from src/test/test_bitcoin.cpp for example, which contains framework/utility code but no real tests.

Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously.

This is an alternative to #12831.

practicalswift · 2018-04-09T20:51:09Z

test/util/bitcoin-unit-tests.sh

+PROCESSES=4
+DIR="$(dirname "$0")"
+
+cat $DIR/../../src/{,wallet/,qt/}test/*.cpp | fgrep BOOST_FIXTURE_TEST_SUITE | cut -d '(' -f 2 | cut -d ',' -f 1 | shuf | xargs -n 1 -P $PROCESSES -I "{}" -- bash "$DIR/bitcoin-unit-test.sh" "{}/*"


Nit: Double quote to prevent globbing and word splitting :-)

cat "$DIR"/../../src/{,wallet/,qt/}test/*.cpp

Fixed (the files are now passed in from Makefile, as suggested by Cory).

jamesob · 2018-04-09T20:54:12Z

test/util/bitcoin-unit-tests.sh

@@ -0,0 +1,6 @@
+#!/bin/bash
+
+PROCESSES=4


Would it make sense just to do xargs ... -P 0?

I don't like hardcoding the parallellism here, but with -P 0 my system will run all tests simultaneously - that's a bit scary w.r.t. resource usage.

practicalswift · 2018-04-09T21:02:36Z

src/Makefile.test.include

@@ -151,6 +150,8 @@ bitcoin_test_clean : FORCE
 	rm -f $(CLEAN_BITCOIN_TEST) $(test_test_bitcoin_OBJECTS) $(TEST_BINARY)

 check-local:
+	@echo "Running test/util/bitcoin-unit-tests.py..."


s/.py/.sh/ :-)

practicalswift · 2018-04-09T21:08:15Z

test/util/bitcoin-unit-tests.sh

+PROCESSES=4
+DIR="$(dirname "$0")"
+
+cat $DIR/../../src/{,wallet/,qt/}test/*.cpp | fgrep BOOST_FIXTURE_TEST_SUITE | cut -d '(' -f 2 | cut -d ',' -f 1 | shuf | xargs -n 1 -P $PROCESSES -I "{}" -- bash "$DIR/bitcoin-unit-test.sh" "{}/*"


shuf is not available under OS X by default. Is sort -R sufficiently portable?

practicalswift · 2018-04-09T21:31:04Z

Strong concept ACK.

I like the straightforwardness and compactness of this solution vs. the previous attempt at parallelism.

Do you have any speedup measurements?

practicalswift · 2018-04-09T21:33:00Z

src/Makefile.test.include

@@ -151,6 +150,8 @@ bitcoin_test_clean : FORCE
 	rm -f $(CLEAN_BITCOIN_TEST) $(test_test_bitcoin_OBJECTS) $(TEST_BINARY)

 check-local:
+	@echo "Running unit tests..."
+	@$(top_builddir)/test/util/bitcoin-unit-tests.sh $(BITCOIN_TESTS)
 	@echo "Running test/util/bitcoin-util-test.py..."


Intentionally keeping bitcoin-util-test.py in there?

These are unrelated tests.

Oh, ofc – my bad. Sorry :-)

sipa · 2018-04-09T21:37:44Z

@practicalswift 48 minutes (when lucky) to 23 minutes for the win32 test in Travis.

maflcko · 2018-04-09T21:43:27Z

48 minutes (when lucky) to 23 minutes for the win32 test in Travis.

🎉

maflcko · 2018-04-09T21:45:27Z

utACK e764ddcab74755274856cf217cdeb83e2d5d9bc0

practicalswift · 2018-04-09T21:46:17Z

@sipa Wow! That will help us get rid of those nasty win32 Travis timeouts! :-)

practicalswift · 2018-04-09T21:49:29Z

utACK e764ddcab74755274856cf217cdeb83e2d5d9bc0

My shell nits on this PR would have been communicated automatically (and hopefully fixed pre human review) if shellcheck linting was enabled in Travis. Please help review #12871 if you find shellcheck valuable :-)

theuni · 2018-04-09T22:04:31Z

48 minutes (when lucky) to 23 minutes for the win32 test in Travis.

Nice! Reviewing.

theuni · 2018-04-09T22:07:36Z

cory@Macbook:~/dev/bitcoin/src$ sort -R
sort: invalid option -- R

:(

practicalswift · 2018-04-09T22:09:15Z

@theuni Oh, that's bad! What version of sort are you using?

FWIW:

$ sort --version
2.3-Apple (99)
$ (echo A; echo B) | sort -R
B
A
$ (echo A; echo B) | sort -R
A
B

sipa · 2018-04-09T22:11:10Z

If the random sorting is an issue we can just sort alphabetically. This would also let us do things like indicate which tests are big in their name for example, to put them earlier in the list.

theuni · 2018-04-09T22:15:27Z

@sipa The profile dir for tests is defined as:

pathTemp = fs::temp_directory_path() / strprintf("test_bitcoin_%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(100000)));

As GetTime() has only second-level resolution, we may want to bump up the rand range a bit.

practicalswift · 2018-04-09T22:17:07Z

@sipa What about using awk?

$ (echo A; echo B) | awk 'BEGIN { srand(); } { printf("%.10f %s\n", rand(), $0); }' | \
      sort | cut -f2- -d' '
A
B
$ (echo A; echo B) | awk 'BEGIN { srand(); } { printf("%.10f %s\n", rand(), $0); }' | \
      sort | cut -f2- -d' '
B
A

sipa · 2018-04-09T22:25:45Z

Removed random sorting (made it alphabetical), reordered the Travis tests from slow to fast (except the doc test which goes still first for fast-fail), and increased the test temp directory entropy.

practicalswift · 2018-04-09T22:27:54Z

utACK aa9be23327db7a12760f250e7261b5f6be6f9de0

sipa · 2018-04-09T22:28:34Z

Why the reordering of Travis builds? I'm not opposing it – just curious :-)

In order to maximally exploit parallelism (of Travis jobs) you generally want to start the longest-running ones to start first. Sometimes not all jobs are started simultaneously (based on other load), in which case this matters in reducing the overall wall clock time spent between pushing and getting a final Travis result.

theuni · 2018-04-10T01:08:02Z

Reviewers: Sorry for the interruption, see the new approach here.

I was discussing this earlier with @sipa and wanted to see if we could get this to work while preserving the make jobs. It turned out to be substantially simpler than what was in the PR already, and I guess @sipa doesn't hate it too much :)

sipa · 2018-04-10T01:24:26Z

Totally changed the approach, integrating everything into makefiles rather than creating external runner scripts (thanks @theuni). See updated PR description.

jamesob · 2018-04-10T02:17:45Z

Tested ACK 7ef9cd8

Seeing a mild improvement locally 🎉

201804_parunit

make -j 4 check 282.46s user 14.91s system 252% cpu 1:57.88 total

master

make -j 4 check 193.71s user 10.10s system 129% cpu 2:37.31 total

laanwj · 2018-04-10T07:14:29Z

Totally changed the approach, integrating everything into makefiles rather than creating external runner scripts (thanks @theuni). See updated PR description.

Heh you implemented my IRC joke. I'm surprised that this works out to an elegant approach.
But it's really neat - will test.

After already having built all, including the test executables, and commenting out secp256k1 and univalue tests (which take up significant time and aren't relevant here):

user@ishtar:~/src/bitcoin/build$ time make -j10 check
real    1m6.551s
user    1m8.388s
sys     0m2.392s

(12926)user@ishtar:~/src/bitcoin/build$ time make -j10 check
real    0m23.140s
user    1m4.544s
sys     0m8.616s

Almost three times speed-up (on an 8-core machine). Nice!

practicalswift · 2018-04-10T09:23:56Z

The new version is even better!

Nice to avoid having two settings for concurrency (make -j and shell script variable).

kallewoof · 2018-04-10T09:39:36Z

Seeing about 2x improvement. Tested ACK

master:         make -j10 check   97.74s user 52.14s system 138% cpu 1:48.19  total
201804_parunit: make -j10 check  109.30s user 16.73s system 219% cpu   57.482 total

jonasschnelli · 2018-04-10T12:11:39Z

Tested ACK 7ef9cd8

OSX 10.13.4 (2.9 GHz Intel Core i7): time make -j5 V=1 check

This PR:

real	1m28.438s
user	3m42.053s
sys	0m45.473s

versus master:

real	3m5.687s
user	2m57.911s
sys	0m46.390s

@theuni

7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille) f6dfb0f Reorder travis builds (Pieter Wuille) 156db42 tests: run tests in parallel (Cory Fields) 66f3255 tests: split up actual tests and helper files (Cory Fields) Pull request description: This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32). This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure. Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests. Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously. This is an alternative to #12831. Tree-SHA512: 9f82eb4ade14ac859618da533c7d9df2aa9f5592a076dcc4939beeffd109eda33f7d5480d8f50c0d8b23bf3099759e9f3a2d4c78efb5b66b04569b39b354c185

Currently unit tests can not be run in parallel. This prevents us from speeding up ci builds (see bitcoin/bitcoin#12926, #865) and won't work with the updated build definitions from bitcoin 0.17 (#860). This is a regression which was introduced in #793. This pull request fixes #861. The problem is that the `StateDB` component does something actively when being initialized (not something a component should do), which is to access disk and so on, which can fail. Since there is a `UnitEInjector` for every `BasicTestingSetup` running the unit tests in parallel creates a database _in the same directory_ per unit tests suite which clashes and breaks and throws and aborts and dies. This can be prevented by using an in-memory database, but the problem shifts: How to get that configuration in? I did not want to expose this as a user-definable setting and also not as a chain parameter, so these two configuration options are not available. Which is why I created `UnitEInjectorConfiguration`. This is something which should not be necessary as usually you would not expose the same module as in production in unit tests but use a subset or just the mocked version, but since the rest of bitcoin is not in well-defined components and accesses them using `GetComponent` a global injector has to be available in some tests. The change ultimately does not affect the prod version of things, but the tests, which inject a different `UnitEInjectorConfiguration`. I made this a `struct` rather then just a flag `unit_tests` such that it is extensible in case we have other cases like this. In order to make the injector be able to access fields from it's own instance in the definition of a component I altered the definition of `UNMANAGED_COMPONENT` a bit. I would like to point out that these initialization issues would also have happened with an Application class (#723), as that is exactly what `UnitEInjector` is, just you would inject things manually. Signed-off-by: Julian Fleischer <julian@thirdhash.com>

@theuni

This is a backport of bitcoin/bitcoin#12926, which unveils how master is currently broken as reported in #861 Here is the original description: > This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32). > > This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure. > > Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests. For bitcoin this improved in quite a nice speedup for builds (especially the Win32 unit test only one): > OSX 10.13.4 (2.9 GHz Intel Core i7): `time make -j5 V=1 check` > > This PR: > > ``` > real 1m28.438s > user 3m42.053s > sys 0m45.473s > ``` > > versus master: > > ``` > real 3m5.687s > user 2m57.911s > sys 0m46.390s > ``` Signed-off-by: Julian Fleischer <julian@thirdhash.com>

@theuni

7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille) f6dfb0f Reorder travis builds (Pieter Wuille) 156db42 tests: run tests in parallel (Cory Fields) 66f3255 tests: split up actual tests and helper files (Cory Fields) Pull request description: This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32). This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure. Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests. Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously. This is an alternative to bitcoin#12831. Tree-SHA512: 9f82eb4ade14ac859618da533c7d9df2aa9f5592a076dcc4939beeffd109eda33f7d5480d8f50c0d8b23bf3099759e9f3a2d4c78efb5b66b04569b39b354c185

@theuni

7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille) f6dfb0f Reorder travis builds (Pieter Wuille) 156db42 tests: run tests in parallel (Cory Fields) 66f3255 tests: split up actual tests and helper files (Cory Fields) Pull request description: This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32). This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure. Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests. Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously. This is an alternative to bitcoin#12831. Tree-SHA512: 9f82eb4ade14ac859618da533c7d9df2aa9f5592a076dcc4939beeffd109eda33f7d5480d8f50c0d8b23bf3099759e9f3a2d4c78efb5b66b04569b39b354c185

@theuni

7ef9cd8 Increase entropy in test temp directory name (Pieter Wuille) f6dfb0f Reorder travis builds (Pieter Wuille) 156db42 tests: run tests in parallel (Cory Fields) 66f3255 tests: split up actual tests and helper files (Cory Fields) Pull request description: This runs the unit tests (`src/test/test_bitcoin`) in 4 separate simultaneous processes, significantly speeding up some Travis runs (over 2x for win32). This uses an approach by @theuni that relies on `make` as the mechanism for distributing tests over processes (through `-j`). For every test .cpp file, we search for `BOOST_FIXTURE_TEST_SUITE` or `BOOST_AUTO_TEST_SUITE`, and then invoke the test binary for just that suite (using `-t`). The (verbose) output is stored in a temporary file, and only shown in the case of failure. Some makefile reshuffling is necessary to avoid trying to run tests from `src/test/test_bitcoin.cpp` for example, which contains framework/utility code but no real tests. Finally, order the Travis jobs from slow to fast (apart from the arm/doc job which goes first, for fast failure). This should help reducing the total wall clock time before opening a PR and finishing Travis, in case where not all jobs are started simultaneously. This is an alternative to bitcoin#12831. Tree-SHA512: 9f82eb4ade14ac859618da533c7d9df2aa9f5592a076dcc4939beeffd109eda33f7d5480d8f50c0d8b23bf3099759e9f3a2d4c78efb5b66b04569b39b354c185

practicalswift reviewed Apr 9, 2018

View reviewed changes

jamesob reviewed Apr 9, 2018

View reviewed changes

practicalswift reviewed Apr 9, 2018

View reviewed changes

sipa force-pushed the 201804_parunit branch from 242fe7c to e764ddc Compare April 9, 2018 21:20

practicalswift reviewed Apr 9, 2018

View reviewed changes

practicalswift mentioned this pull request Apr 9, 2018

[WIP] Run unit tests in parallel #12831

Closed

sipa force-pushed the 201804_parunit branch from e764ddc to aa9be23 Compare April 9, 2018 22:24

fanquake added the Tests label Apr 9, 2018

theuni and others added 4 commits April 9, 2018 19:55

tests: split up actual tests and helper files

66f3255

tests: run tests in parallel

156db42

Reorder travis builds

f6dfb0f

Increase entropy in test temp directory name

7ef9cd8

sipa force-pushed the 201804_parunit branch from aa9be23 to 7ef9cd8 Compare April 10, 2018 00:30

laanwj merged commit 7ef9cd8 into bitcoin:master Apr 10, 2018

ryanofsky mentioned this pull request Apr 10, 2018

Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection #11640

Merged

ajtowns mentioned this pull request Apr 10, 2018

Use winehq debs for win32 tests #12931

Closed

sickpig mentioned this pull request May 23, 2018

[PORT] Run unit tests in parallel BitcoinUnlimited/BitcoinUnlimited#1097

Merged

This was referenced Mar 28, 2019

unit tests can no longer be run in parallel dtr-org/unit-e#861

Closed

Parallel make check dtr-org/unit-e#865

Merged

Allow unit tests being run in parallel again dtr-org/unit-e#870

Merged

markblundeberg mentioned this pull request Jul 3, 2019

Fix "gmake check" under OpenBSD 6.3 (probably *BSD): Avoid using GNU grep specific regexp handling #13355

Merged

xdustinface added a commit to xdustinface/dash that referenced this pull request Apr 14, 2021

More of bitcoin#12926

2b14594

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 14, 2021

More of bitcoin#12926

b929078

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2021

More of bitcoin#12926

7f5e788

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run unit tests in parallel #12926

Run unit tests in parallel #12926

sipa commented Apr 9, 2018 •

edited

practicalswift Apr 9, 2018

sipa Apr 9, 2018 •

edited

jamesob Apr 9, 2018

sipa Apr 9, 2018

practicalswift Apr 9, 2018 •

edited

sipa Apr 9, 2018

practicalswift Apr 9, 2018 •

edited

sipa Apr 9, 2018

practicalswift commented Apr 9, 2018

practicalswift Apr 9, 2018 •

edited

sipa Apr 9, 2018

practicalswift Apr 9, 2018 •

edited

sipa commented Apr 9, 2018

maflcko commented Apr 9, 2018

maflcko commented Apr 9, 2018

practicalswift commented Apr 9, 2018 •

edited

practicalswift commented Apr 9, 2018 •

edited

theuni commented Apr 9, 2018

theuni commented Apr 9, 2018

practicalswift commented Apr 9, 2018 •

edited

sipa commented Apr 9, 2018

theuni commented Apr 9, 2018

practicalswift commented Apr 9, 2018 •

edited

sipa commented Apr 9, 2018

practicalswift commented Apr 9, 2018

sipa commented Apr 9, 2018 •

edited

theuni commented Apr 10, 2018

sipa commented Apr 10, 2018

jamesob commented Apr 10, 2018

laanwj commented Apr 10, 2018 •

edited

practicalswift commented Apr 10, 2018

kallewoof commented Apr 10, 2018 •

edited

jonasschnelli commented Apr 10, 2018

		@@ -0,0 +1,6 @@
		#!/bin/bash

		PROCESSES=4

Run unit tests in parallel #12926

Run unit tests in parallel #12926

Conversation

sipa commented Apr 9, 2018 • edited

practicalswift Apr 9, 2018

Choose a reason for hiding this comment

sipa Apr 9, 2018 • edited

Choose a reason for hiding this comment

jamesob Apr 9, 2018

Choose a reason for hiding this comment

sipa Apr 9, 2018

Choose a reason for hiding this comment

practicalswift Apr 9, 2018 • edited

Choose a reason for hiding this comment

sipa Apr 9, 2018

Choose a reason for hiding this comment

practicalswift Apr 9, 2018 • edited

Choose a reason for hiding this comment

sipa Apr 9, 2018

Choose a reason for hiding this comment

practicalswift commented Apr 9, 2018

practicalswift Apr 9, 2018 • edited

Choose a reason for hiding this comment

sipa Apr 9, 2018

Choose a reason for hiding this comment

practicalswift Apr 9, 2018 • edited

Choose a reason for hiding this comment

sipa commented Apr 9, 2018

maflcko commented Apr 9, 2018

maflcko commented Apr 9, 2018

practicalswift commented Apr 9, 2018 • edited

practicalswift commented Apr 9, 2018 • edited

theuni commented Apr 9, 2018

theuni commented Apr 9, 2018

practicalswift commented Apr 9, 2018 • edited

sipa commented Apr 9, 2018

theuni commented Apr 9, 2018

practicalswift commented Apr 9, 2018 • edited

sipa commented Apr 9, 2018

practicalswift commented Apr 9, 2018

sipa commented Apr 9, 2018 • edited

theuni commented Apr 10, 2018

sipa commented Apr 10, 2018

jamesob commented Apr 10, 2018

201804_parunit

master

laanwj commented Apr 10, 2018 • edited

practicalswift commented Apr 10, 2018

kallewoof commented Apr 10, 2018 • edited

jonasschnelli commented Apr 10, 2018

sipa commented Apr 9, 2018 •

edited

sipa Apr 9, 2018 •

edited

practicalswift Apr 9, 2018 •

edited

practicalswift Apr 9, 2018 •

edited

practicalswift Apr 9, 2018 •

edited

practicalswift Apr 9, 2018 •

edited

practicalswift commented Apr 9, 2018 •

edited

practicalswift commented Apr 9, 2018 •

edited

practicalswift commented Apr 9, 2018 •

edited

practicalswift commented Apr 9, 2018 •

edited

sipa commented Apr 9, 2018 •

edited

laanwj commented Apr 10, 2018 •

edited

kallewoof commented Apr 10, 2018 •

edited