Slow unit tests delay functional tests and leave CPU unused

maflcko commented at 1:07 pm on June 18, 2025: member

When running the unit and functional tests under a sanitizer (as done in some CI tasks), the CPU usage could be better. Currently, one or two slow running unit tests idle along on a single CPU thread.

Can be seen in this graph, which shows an initial spike during compilation, then the slow unit tests, and finally the functional test CPU usage:

It would be nice to somehow fix this. Options are:

Speed up the unit tests (not sure how easy this is, or if this is possible at all)
Run the functional tests along with the ctest-tests in parallel somehow
Something else?

maflcko added the label Tests on Jun 18, 2025

willcl-ark commented at 1:14 pm on June 18, 2025: member

I have noticed similar when investigating tangential Ci changes. It is possible to run more unit tests in parallel, but we still always wait for the slowest test.

If I recall it’s a secp256 test, and there was perhaps a suggestion to disable it (mostly), or have it run weekly or something?

maflcko commented at 1:38 pm on June 18, 2025: member

Looking at https://github.com/bitcoin/bitcoin/actions/runs/15714959524/job/44282297642#step:6:4481, it seems there are several affected tests:

0...
1135/142 Test  [#66](/bitcoin-bitcoin/66/): net_tests ............................   Passed  243.14 sec
2136/142 Test [#139](/bitcoin-bitcoin/139/): wallet_tests .........................   Passed  108.31 sec
3137/142 Test [#138](/bitcoin-bitcoin/138/): wallet_crypto_tests ..................   Passed  116.03 sec
4138/142 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_noverify_tests .............   Passed  426.46 sec
5139/142 Test [#129](/bitcoin-bitcoin/129/): coinselector_tests ...................   Passed  268.56 sec
6140/142 Test   [#9](/bitcoin-bitcoin/9/): bench_sanity_check ...................   Passed  487.70 sec
7141/142 Test  [#32](/bitcoin-bitcoin/32/): coins_tests ..........................   Passed  559.64 sec
8142/142 Test   [#6](/bitcoin-bitcoin/6/): secp256k1_tests ......................   Passed  590.82 sec

edit:

Tsan:

0[20:37:36.776] 136/142 Test  [#32](/bitcoin-bitcoin/32/): coins_tests ..........................   Passed  122.51 sec
1[20:37:38.388] 137/142 Test  [#30](/bitcoin-bitcoin/30/): checkqueue_tests .....................   Passed  124.34 sec
2[20:37:46.297] 138/142 Test [#139](/bitcoin-bitcoin/139/): wallet_tests .........................   Passed   29.87 sec
3[20:38:40.235] 139/142 Test   [#9](/bitcoin-bitcoin/9/): bench_sanity_check ...................   Passed  193.33 sec
4[20:38:42.459] 140/142 Test [#129](/bitcoin-bitcoin/129/): coinselector_tests ...................   Passed   98.51 sec
5[20:42:20.591] 141/142 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_noverify_tests .............   Passed  413.70 sec
6[20:48:11.703] 142/142 Test   [#6](/bitcoin-bitcoin/6/): secp256k1_tests ......................   Passed  764.81 sec

msan:

0[13:02:32.115] 134/141 Test  [#31](/bitcoin-bitcoin/31/): coins_tests ..........................   Passed   98.23 sec
1[13:02:40.482] 135/141 Test [#138](/bitcoin-bitcoin/138/): wallet_tests .........................   Passed   22.38 sec
2[13:02:53.347] 136/141 Test [#114](/bitcoin-bitcoin/114/): txvalidationcache_tests ..............   Passed   67.72 sec
3[13:03:29.497] 137/141 Test [#128](/bitcoin-bitcoin/128/): coinselector_tests ...................   Passed   78.19 sec
4[13:03:32.923] 138/141 Test  [#29](/bitcoin-bitcoin/29/): checkqueue_tests .....................   Passed  163.40 sec
5[13:04:12.828] 139/141 Test   [#8](/bitcoin-bitcoin/8/): bench_sanity_check ...................   Passed  207.55 sec
6[13:05:08.881] 140/141 Test   [#5](/bitcoin-bitcoin/5/): secp256k1_noverify_tests .............   Passed  263.61 sec
7[13:08:27.015] 141/141 Test   [#6](/bitcoin-bitcoin/6/): secp256k1_tests ......................   Passed  461.74 sec

willcl-ark commented at 3:08 pm on June 19, 2025: member

It would be helpful to know when we should run these secp256k1 tests in this repo. I think they are run in the libsecp repo as part of CI based on make "$BUILD" defaulting to make check, but I’ve not verified too closely.

I am not sure I see any point to re-running them here on every change. What sounds more sensible to me is to build and run them on subtree updates only. I have a demo branch adding such a CI job which I can tidy and open as a PR, if something like this is wanted?

This perhaps won’t “fix” this issue entirely, but should make a difference of a few minutes per job on a few of our jobs.

maflcko commented at 3:43 pm on June 19, 2025: member

I am not sure I see any point to re-running them here on every change. What sounds more sensible to me is to build and run them on subtree updates only. I have a demo branch adding such a CI job which I can tidy and open as a PR, if something like this is wanted?

No objection from my side.

This perhaps won’t “fix” this issue entirely

I guess the easiest fix would be to call ctest --test-dir ./bld-cmake --show-only=human from the functional test runner and then run those listed in separate processes (when an option like --run-ctest-tests) is given?

willcl-ark commented at 9:33 am on June 20, 2025: member

I opened a draft for disabling the secp256 tests by default, running them only on subtree updates.

Regarding running unit tests from the functional tests, the simplest would probably be to run ctest --test-dir build -j$(nproc) using a new process from multiprocessing, then dump stdout/stderr in the results section and forward on the return code (if non-zero) to the runner.

But it sounds like you are proposing something more like enumerating all tests and running them singly from the runner using the existing infrastructure?

maflcko commented at 9:37 am on June 20, 2025: member

yeah, I am not sure if it is fine to double prarallelize (nproc unit tests at the same time with nproc functional tests), but I guess it just means the early launced functional tests will be a bit slower, which should be fine

willcl-ark commented at 9:55 am on June 20, 2025: member

It appears to work on my machine OK running them both with nproc in parallel. In fact on my 16 core machine I can run ctest at -j16 and test_runner.py --jobs=20 together without issue.

Just to be clear, would we want the ctest output included and integrated in the test_runner results section? If so this will I think involve parsing the ctest output (and therefore more code). If we are happy just to run it using multiprocess and dump stdout/err at the end, and record the exit code, the change will be a fair bit smaller (but the output possibly messier!)

ajtowns commented at 0:23 am on July 17, 2025: contributor

cf #32945 maybe?

Changing test_runner.py from python to cmake logic might make running the functional and unit tests in parallel more plausible?

maflcko commented at 11:38 am on July 17, 2025: member

cf #32945 maybe?

Can’t hurt, but depending on the sanitizer, it is a different test that is the slowest: #32770 (comment) so the approach would have to be done for other tests as well.

Submitted #33000 for the ci in the meantime.

maflcko commented at 8:46 am on August 6, 2025: member

bench_sanity_check could make sense to split up as well. With sanitizers enabled, it takes several minutes, even on fast hardware. See #28676 (comment). So I created https://github.com/bitcoin/bitcoin/pull/33142

fanquake commented at 9:57 am on August 6, 2025: member

coinselector_tests is a good candidate for speedup, under Valgrind, it is the slowest after bench_sanity_check:

0141/144 Test  [#78](/bitcoin-bitcoin/78/): random_tests .........................   Passed  227.62 sec
1142/144 Test  [#30](/bitcoin-bitcoin/30/): coins_tests_dbbase ...................   Passed  336.61 sec
2143/144 Test [#130](/bitcoin-bitcoin/130/): coinselector_tests ...................   Passed  459.29 sec
3144/144 Test   [#6](/bitcoin-bitcoin/6/): bench_sanity_check ...................   Passed  793.02 sec

Same for wallet_miniscript.py in the functional tests. Under Valgrind, it is the slowest test by at least 2x any other test. tool_signet_miner.py and wallet_taproot.py are also very slow.

murchandamus commented at 8:30 pm on August 6, 2025: contributor

I started looking into that as it aligns well with one of my projects to migrate the coinselector_tests to an updated test suite coinselection_tests.

murchandamus commented at 10:16 pm on August 6, 2025: contributor

In the context of coinselector_tests being slow, I found looking at some flamegraphs that much of the time is spent on Knapsack and BnB. I have a rewrite of BnB open, that I would expect to improve BnB’s performance (and thus improve this test’s performance, in case someone is interested to review) and I have been prototyping another coin selection algorithm that I hope will bring the last missing piece to obsolete Knapsack (consolidatory behavior at low feerates) for which I have recently refurbished the coin selection simulation framework and have been running some exploratory simulations. The migration of the first part of the coinselector_tests to coinselection_tests was recently merged via #29532, so I hope to get around to opening a PR for the next part soonish.

fanquake commented at 10:27 am on September 17, 2025: member

Same for wallet_miniscript.py in the functional tests. Under Valgrind, it is the slowest test by at least 2x any other test.

Seems to be ~4 times slower than the next slowest test: #33411 (comment).

maflcko commented at 7:33 am on October 16, 2025: member

On the general topic, there was commit https://github.com/bitcoin/bitcoin/commit/636b611781154f32c8d7ae8ea301d12b1464c151, so someone could some time in the future check if it helps.

furszy commented at 1:35 pm on October 16, 2025: member

On the libsecp notes, since https://github.com/bitcoin-core/secp256k1/pull/1734 we can parallelize tests. Tagging @hebasto as he had a patch to do it.

hebasto commented at 2:30 pm on October 16, 2025: member

On the libsecp notes, since bitcoin-core/secp256k1#1734 we can parallelize tests. Tagging @hebasto as he had a patch to do it.

See https://github.com/bitcoin-core/secp256k1/pull/1760.

achow101 referenced this in commit 0ad4376a49 on Jan 5, 2026

maflcko commented at 2:14 pm on January 6, 2026: member

#33142 was merged and the same approach could trivially be used for the unit tests, but there the motivating QA issues listed in the pulls description are less pressing for the unit tests right now, so I’ll wait a few months before working on this again. In the meantime, /pull/33483 could be picked up.

Slow unit tests delay functional tests and leave CPU unused #32770