Run unit tests in parallel

sipa commented at 8:35 pm on April 9, 2018: member

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.

in test/util/bitcoin-unit-tests.sh:6 in 242fe7c19a outdated

0@@ -0,0 +1,6 @@
1+#!/bin/bash
2+
3+PROCESSES=4
4+DIR="$(dirname "$0")"
5+
6+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" "{}/*"

practicalswift commented at 8:51 pm on April 9, 2018:

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

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

practicalswift commented at 9:08 pm on April 9, 2018:

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

sipa commented at 9:27 pm on April 9, 2018:

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

sipa commented at 9:27 pm on April 9, 2018:

Fixed.

in test/util/bitcoin-unit-tests.sh:3 in 242fe7c19a outdated

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

jamesob commented at 8:54 pm on April 9, 2018:

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

sipa commented at 9:27 pm on April 9, 2018:

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.

in src/Makefile.test.include:153 in 242fe7c19a outdated

149@@ -151,6 +150,8 @@ bitcoin_test_clean : FORCE
150 	rm -f $(CLEAN_BITCOIN_TEST) $(test_test_bitcoin_OBJECTS) $(TEST_BINARY)
151 
152 check-local:
153+	@echo "Running test/util/bitcoin-unit-tests.py..."

practicalswift commented at 9:02 pm on April 9, 2018:

s/.py/.sh/ :-)

sipa commented at 9:27 pm on April 9, 2018:

Fixed.

sipa force-pushed on Apr 9, 2018

practicalswift commented at 9:31 pm on April 9, 2018: contributor

Strong concept ACK.

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

Do you have any speedup measurements?

in src/Makefile.test.include:157 in e764ddcab7 outdated

149@@ -151,6 +150,8 @@ bitcoin_test_clean : FORCE
150 	rm -f $(CLEAN_BITCOIN_TEST) $(test_test_bitcoin_OBJECTS) $(TEST_BINARY)
151 
152 check-local:
153+	@echo "Running unit tests..."
154+	@$(top_builddir)/test/util/bitcoin-unit-tests.sh $(BITCOIN_TESTS)
155 	@echo "Running test/util/bitcoin-util-test.py..."

practicalswift commented at 9:33 pm on April 9, 2018:

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

sipa commented at 9:37 pm on April 9, 2018:

These are unrelated tests.

practicalswift commented at 9:44 pm on April 9, 2018:

Oh, ofc – my bad. Sorry :-)

sipa commented at 9:37 pm on April 9, 2018: member

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

MarcoFalke commented at 9:43 pm on April 9, 2018: member

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

:tada:

MarcoFalke commented at 9:45 pm on April 9, 2018: member

utACK e764ddcab74755274856cf217cdeb83e2d5d9bc0

practicalswift commented at 9:46 pm on April 9, 2018: contributor

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

practicalswift commented at 9:49 pm on April 9, 2018: contributor

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 commented at 10:04 pm on April 9, 2018: member

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

Nice! Reviewing.

theuni commented at 10:07 pm on April 9, 2018: member

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

:(

practicalswift commented at 10:09 pm on April 9, 2018: contributor

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

FWIW:

0$ sort --version
12.3-Apple (99)
2$ (echo A; echo B) | sort -R
3B
4A
5$ (echo A; echo B) | sort -R
6A
7B

sipa commented at 10:11 pm on April 9, 2018: member

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 commented at 10:15 pm on April 9, 2018: member

@sipa The profile dir for tests is defined as:

0pathTemp = 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 commented at 10:17 pm on April 9, 2018: contributor

@sipa What about using awk?

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

sipa force-pushed on Apr 9, 2018

sipa commented at 10:25 pm on April 9, 2018: member

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.

fanquake added the label Tests on Apr 9, 2018

practicalswift commented at 10:27 pm on April 9, 2018: contributor

utACK aa9be23327db7a12760f250e7261b5f6be6f9de0

sipa commented at 10:28 pm on April 9, 2018: member

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.

tests: split up actual tests and helper files 66f32551bd

tests: run tests in parallel 156db42c3f

Reorder travis builds f6dfb0f504

Increase entropy in test temp directory name 7ef9cd8491

sipa force-pushed on Apr 10, 2018

theuni commented at 1:08 am on April 10, 2018: member

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 commented at 1:24 am on April 10, 2018: member

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

jamesob commented at 2:17 am on April 10, 2018: member

Tested ACK https://github.com/bitcoin/bitcoin/pull/12926/commits/7ef9cd8491185af26295b3501fca9d2a49379364

Seeing a mild improvement locally :tada:

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 commented at 7:14 am on April 10, 2018: member

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):

0user@ishtar:~/src/bitcoin/build$ time make -j10 check
1real    1m6.551s
2user    1m8.388s
3sys     0m2.392s
4
5(12926)user@ishtar:~/src/bitcoin/build$ time make -j10 check
6real    0m23.140s
7user    1m4.544s
8sys     0m8.616s

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

practicalswift commented at 9:23 am on April 10, 2018: contributor

The new version is even better!

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

kallewoof commented at 9:39 am on April 10, 2018: member

Seeing about 2x improvement. Tested ACK

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

jonasschnelli commented at 12:11 pm on April 10, 2018: contributor

Tested ACK 7ef9cd8491185af26295b3501fca9d2a49379364

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

This PR:

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

versus master:

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

laanwj merged this on Apr 10, 2018

laanwj closed this on Apr 10, 2018

laanwj referenced this in commit dd1ca9e0b3 on Apr 10, 2018

scravy referenced this in commit db2dbc9090 on Apr 1, 2019

scravy referenced this in commit ccdeb51be5 on Apr 1, 2019

PastaPastaPasta referenced this in commit e8962ff953 on Apr 13, 2021

xdustinface referenced this in commit 2b145943db on Apr 14, 2021

PastaPastaPasta referenced this in commit b929078ec8 on Apr 14, 2021

PastaPastaPasta referenced this in commit 7f5e7885fb on Apr 15, 2021

PastaPastaPasta referenced this in commit 460c1ee359 on Apr 17, 2021

kittywhiskers referenced this in commit 53254c5dbf on Apr 23, 2021

MarcoFalke locked this on Sep 8, 2021

Run unit tests in parallel #12926

201804_parunit

master