[WIP] Run unit tests in parallel #12831

maflcko · 2018-03-29T15:47:37Z

Unit tests can be run in parallel on systems with more than one cpu and thus run faster.

Since each test case is run separately from all others, test cases will no longer share the same global variables.

maflcko · 2018-03-29T15:47:44Z

TODO:

Should be run on make check

practicalswift · 2018-03-29T17:11:19Z

Concept ACK

Very nice!

practicalswift · 2018-03-29T17:13:03Z

Perhaps a stupid question but why the need of a static test_list.txt? What would be the disadvantages of generating the test list at run-time?

maflcko · 2018-03-29T17:28:05Z

--list_content is only documented in 1.60.0, so I assume it wouldn't work in previous versions. Currently we support 1.47.0+

http://www.boost.org/doc/libs/1_60_0/libs/test/doc/html/boost_test/utf_reference/rt_param_reference/list_content.html
https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#dependencies

practicalswift · 2018-03-29T20:30:31Z

@MarcoFalke The output from git grep -E '(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_CASE)' -- "src/**.cpp" could perhaps be used to quickly (~14 ms on my machine) generate the list of available tests?

practicalswift · 2018-03-29T20:39:18Z

If we go with the static text file test_list.txt I suggest adding a script contrib/devtools/lint-test_list.sh which checks that the list of tests in test_list.txt is in sync with the list of tests given by src/test/test_bitcoin --list_content.

That way Travis would automatically catch the case where someone adds a test and forgets to manually run src/test/parallel.py --write_list (which requires running Boost.Test 1.60.0 or above) and commit the changes made to test_list.txt after adding a new test.

conscott · 2018-03-30T05:08:48Z

src/test/parallel.py

@@ -0,0 +1,864 @@
+#!/usr/bin/env python


Should be python3?

conscott · 2018-03-30T05:24:29Z

src/test/parallel.py

+  def __init__(self, output_dir):
+    if sys.stdout.isatty():
+      # stdout needs to be unbuffered since the output is interactive.
+      sys.stdout = os.fdopen(sys.stdout.fileno(), 'w', 0)


This seems to be fine in python 2.7 but is a problem in 3.x

ValueError: can't have unbuffered text I/O

The 0 buffer is only valid for byte streams in python 3, from docs the default buffer policy for interactive text files is line buffering, which is probably fine.

conscott · 2018-03-30T05:41:18Z

src/test/test_list.txt

+base58_tests/base58_EncodeBase58
+base58_tests/base58_DecodeBase58
+base64_tests/base64_testvectors
+abc/bech32_tests/bip173_testvectors_valid


not sure why abc/ is appended to path. Getting error:

Test setup error: no test cases matching filter

Removing abc/ lets tests run and pass.

This also poses an interesting problem. If the test case isn't found, its listed as FAILED TESTS, which is somewhat confusing, because bech32_tests/bip173_testvectors_valid would pass, it's just the path listed is wrong.

Maybe the initial reader of FILE_TEST_LIST can verify the path exists before handing it off to workers.

conscott · 2018-03-30T05:52:47Z

src/test/parallel.py

+    parser.print_usage()
+    sys.exit(1)
+
+  if options.shard_count < 1:


Nit: if you are going to verify the input is valid, you can check some other fields as well

Right now you could input a negative for workers, repeat, timeout, etc. without complaint

conscott · 2018-03-30T06:01:30Z

src/test/parallel.py

+  parser.add_option('--shard_count', type='int', default=1,
+                    help='total number of shards (for sharding test execution '
+                         'between multiple machines)')
+  parser.add_option('--shard_index', type='int', default=0,


Not sure it makes sense to give shard_index a default. I think you want to ensure that shard_count and shard_index are used in combination, so if shard_count is used and options.shard_index is None, you print proper usage. Right now options.shard_index will just default to 0, so you can't tell if it's being used properly with shard_count.

conscott · 2018-03-30T06:15:14Z

src/test/parallel.py

+            task = self.tasks[task_id]
+
+            if self.running_groups is not None:
+              test_group = task.test_name.split('.')[0]


Should this be split('/')[0] ?

Right now task.test_name is a line from src/test/test_list.txt like

bloom_tests/rolling_bloom

So the split will always return that full path, since there is no . - making option.serialize_test_cases just run everything in parallel anyway.

conscott · 2018-03-30T06:19:11Z

src/test/parallel.py

+  parser.add_option('--timeout', type='int', default=None,
+                    help='Interrupt all remaining processes after the given '
+                         'time (in seconds).')
+  parser.add_option('--serialize_test_cases', action='store_true',


I think this option is currently incompatible with sharding, since it shards tests in a round-robin type fashion, thus splitting up test_groups between shards. This is probably desired, but just need to document it, or try to prevent the two options being used in combination.

conscott · 2018-03-30T06:21:41Z

Concept ACK - Tested it out and left some initial feedback. Realize it's WIP so just left broad comments.

maflcko · 2018-04-01T22:21:44Z

Thanks for looking at this! I kept the patches to the gtest-parallel script minimal. Feedback not about my patches should be submitted upstream: https://github.com/google/gtest-parallel

Also, if someone knows more about autotools, help is very much appreciated to make it run on make check.

maflcko · 2018-04-05T15:26:48Z

@theuni Mind to give a Concept ACK/NACK or some general comments?

sipa · 2018-04-05T15:44:23Z

I have an idea for a simpler approach, where you could tell the test binary there are N processes, and which one out of those it is. It would then partition the tests randomly into N groups, and only run one of them.

If there's interest, I'll try to implement that soon.

maflcko · 2018-04-05T15:56:35Z

@sipa That wouldn't help running the most time-expensive test first or avoiding that two expensive tests end up in the same group?

sipa · 2018-04-05T16:15:16Z

@MarcoFalke No, but I think that's an independent problem. If some tests take exorbitantly more time than others, perhaps those tests need to be split up.

maflcko · 2018-04-05T16:52:25Z

See #10026 for an (outdated) list of slow unit tests. I haven't checked how practical it is to split them up, but there will always be tests that run slower compared to others.

maflcko · 2018-04-05T17:20:28Z

The currently longest running test on my machine seems to be "test_big_witness_transaction":

$ python3 ./src/test/parallel.py | tail -3
[285/287] streams_tests/streams_serializedata_xor (47 ms)
[286/287] coinselector_tests/knapsack_solver_test (12060 ms)
[287/287] transaction_tests/test_big_witness_transaction (17931 ms)

maflcko · 2018-04-05T19:58:42Z

Got rid of test_list.txt
Run test suites in parallel instead of test cases

practicalswift · 2018-04-05T20:07:23Z

Repeating concept ACK

Automatic linting comment: transform_boost_output_to_test_list is unused now? :-)

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

NACK. Prefering #12926 :-)

@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

As of commit 9ae552468cf096cb281d1ab7c87d9baea56e86c9 google/gtest-parallel@9ae5524

maflcko · 2018-04-10T16:08:40Z

Rebased

@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

fanquake added the Tests label Mar 29, 2018

conscott reviewed Mar 30, 2018

View reviewed changes

src/test/parallel.py

@@ -0,0 +1,864 @@

#!/usr/bin/env python

Copy link

Contributor

conscott Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be python3?

conscott reviewed Mar 30, 2018

View reviewed changes

maflcko force-pushed the Mf1803-qaUnitParallel branch from 4292954 to 2697e9f Compare March 30, 2018 14:51

maflcko added the Up for grabs label Apr 5, 2018

maflcko force-pushed the Mf1803-qaUnitParallel branch 3 times, most recently from fd67998 to 4209ec2 Compare April 5, 2018 17:16

maflcko force-pushed the Mf1803-qaUnitParallel branch from 2b3062c to ed03835 Compare April 5, 2018 20:31

sipa mentioned this pull request Apr 9, 2018

Run unit tests in parallel #12926

Merged

MarcoFalke added 2 commits April 10, 2018 11:51

test: Add parallel.py from gtest-parallel

8510c7e

As of commit 9ae552468cf096cb281d1ab7c87d9baea56e86c9 google/gtest-parallel@9ae5524

test: Adjust parallel.find_tests for our unit tests

7785663

maflcko force-pushed the Mf1803-qaUnitParallel branch from ed03835 to 7785663 Compare April 10, 2018 16:06

maflcko removed the Up for grabs label Apr 10, 2018

maflcko closed this Apr 10, 2018

maflcko deleted the Mf1803-qaUnitParallel branch April 10, 2018 16:09

HashUnlimited mentioned this pull request Sep 5, 2018

Run unit tests in parallel chaincoin/chaincoin#192

Merged

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

[WIP] Run unit tests in parallel #12831

[WIP] Run unit tests in parallel #12831

Uh oh!

Conversation

maflcko commented Mar 29, 2018

Uh oh!

maflcko commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Mar 29, 2018

Uh oh!

practicalswift commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Mar 29, 2018

Uh oh!

practicalswift commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott Mar 30, 2018

Choose a reason for hiding this comment

Uh oh!

conscott commented Mar 30, 2018

Uh oh!

maflcko commented Apr 1, 2018

Uh oh!

maflcko commented Apr 5, 2018

Uh oh!

sipa commented Apr 5, 2018

Uh oh!

maflcko commented Apr 5, 2018

Uh oh!

sipa commented Apr 5, 2018

Uh oh!

maflcko commented Apr 5, 2018

Uh oh!

maflcko commented Apr 5, 2018

Uh oh!

maflcko commented Apr 5, 2018

Uh oh!

practicalswift commented Apr 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Apr 9, 2018

Uh oh!

maflcko commented Apr 10, 2018

Uh oh!

Uh oh!

maflcko commented Mar 29, 2018 •

edited

Loading

practicalswift commented Mar 29, 2018 •

edited

Loading

practicalswift commented Mar 29, 2018 •

edited

Loading

practicalswift commented Mar 29, 2018 •

edited

Loading

conscott Mar 30, 2018 •

edited

Loading

practicalswift commented Apr 5, 2018 •

edited

Loading