This adds arguments to some of the testing setup constructors for creating an environment without networking and a validation interface. This is useful for improving the performance of the utxo snapshot fuzz test, which constructs a new TestingSetup on each iteration.
Using this slimmed down TestingSetup in future might also make the tests a bit faster when run in aggregate.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko, dergoegge |
| Concept ACK | brunoerg, BrandonOdiwuor |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
36 | + /*chain_type*/ ChainType::REGTEST, 37 | + /*extra_args*/ AddNoLogArgs({}), 38 | + /*coins_db_in_memory*/ true, 39 | + /*block_tree_db_in_memory*/ true, 40 | + /*setup_net*/ false, 41 | + /*setup_validation_interface*/ false)};
Concept ACK. Thanks!
Concept ACK
Rebased 049719b3858f2ae88cc2752ffe240b667c3d2996 -> 598dd9c3df8afbab92f706e1d4ce72bd3fa83e95 (noNetChainTest_0 -> noNetChainTest_1, compare)
Adds more testing options for creating an environment without networking
and a validation interface. This is useful for improving the performance
of the utxo snapshot fuzz test, which constructs a new TestingSetup on
each iteration.
30 | @@ -31,7 +31,14 @@ void initialize_chain() 31 | FUZZ_TARGET(utxo_snapshot, .init = initialize_chain) 32 | { 33 | FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); 34 | - std::unique_ptr<const TestingSetup> setup{MakeNoLogFileContext<const TestingSetup>()}; 35 | + std::unique_ptr<const TestingSetup> setup{ 36 | + MakeNoLogFileContext<const TestingSetup>( 37 | + ChainType::REGTEST, 38 | + TestOpts{ 39 | + .extra_args = AddNoLogArgs({}),
Looks like this is called twice, no?
Updated 598dd9c3df8afbab92f706e1d4ce72bd3fa83e95 -> f46b2202560a76b473e229b77303b8f877c16cac (noNetChainTest_1 -> noNetChainTest_2, compare)
review ACK f46b2202560a76b473e229b77303b8f877c16cac
Seems fine to do. I haven't benchmarked this commit, but I expect the peak memory usage of the coins_view fuzz target to go down. Also, the utxo_snapshot fuzz target may be faster and use less peak memory.
(When I benchmarked an earlier version of this, I didn't find a significant speedup, but I may have done something wrong and the changes here seem useful either way)
Concept ACK
(When I benchmarked an earlier version of this, I didn't find a significant speedup, but I may have done something wrong and the changes here seem useful either way)
Same here. The changes lgtm but I tried to benchmark it and I didn't see any significant improvement. Not sure if I did it right.
Upgrading my review with a test:
FUZZ=utxo_snapshot /usr/bin/time --format='%MKB' ./src/test/fuzz/fuzz -runs=1 ../b-c-qa-assets/fuzz_seed_corpus/utxo_snapshot/ gives: 2MB less peak memory (~1%) and 600 less cov (~10%) (which is desired here, to avoid reporting non-fuzzing coverage in a fuzz target) and no speedup/slowdownFUZZ=coins_view gives: 50MB less peak memory (~25%) and no other changelgtm
51 | @@ -52,6 +52,8 @@ struct TestOpts { 52 | std::vector<const char*> extra_args{}; 53 | bool coins_db_in_memory{true}; 54 | bool block_tree_db_in_memory{true}; 55 | + bool setup_net{true};
nit: A bit weird that this will be available as an option on all testing setups when only TestingSetup and it's derived classes care about it.
I did this on purpose when adding TestOpts, because deriving a nested stack of TestOpt classes would make designated initializers harder, something like { { { .inner = true } }, .outer = false}, instead of just { .inner = true , .outer = false}. (Not sure if there were also compiler warnings in the first case)
utACK f46b2202560a76b473e229b77303b8f877c16cac
test-only pull with two ACKs, rfm?