bench: add benchmark for wallet 'AvailableCoins' function. #25234

Rationale

AvailableCoins is part of several important flows for the wallet; from RPC commands that create transactions like fundrawtransaction, send, walletcreatefundedpsbt, get the available balance, list the available coins with listunspent etc. to GUI connected processes that perform the same or similar actions: tx creation, available balance calculation, present the spendable coins in the coin control dialog.

As we are improving this process in #24699, #25005 and there are more structural changes coming on the way. This benchmark aims to ensure us that, at least, there are no regressions (obviously performance improvements are great but, at least for me, this heads into the direction of having a base metric to compare future structural changes).

Implementation Notes

There are 5 new benchmarks, one per wallet supported output type (LEGACY, P2SH_SEGWIT, BECH32, BECH32M), plus a multi-output-type wallet benchmark which contains outputs from all the descriptor types.

The test, by default, fills-up the wallet with 1k transactions, 2k outputs. Mainly to not consume much time if the user just want to verify that no substantial regressions were introduced. But, my expectation for those who are focused on this process is to use a much higher number locally to really note the differences across commits.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

Concept ACK on benchmarking these, however there have been recent complaints that the benchmarks are slow, and I've noticed these add more fairly slow to run benchmarks:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      359,740,298.60 |                2.78 |    0.0% |     19.79 | `WalletAvailableCoinsMulti`
|      289,759,212.20 |                3.45 |    0.0% |     15.93 | `WalletAvailableCoinsOnlyBech32`
|      559,973,432.00 |                1.79 |    0.0% |     30.80 | `WalletAvailableCoinsOnlyBech32M`
|      196,348,457.20 |                5.09 |    0.0% |     10.80 | `WalletAvailableCoinsOnlyLegacy`
|      304,247,935.40 |                3.29 |    0.0% |     16.73 | `WalletAvailableCoinsOnlyP2SH_SEGWIT`

E.g. a total of 94 seconds to the runtime of bench_bitcoin with default settings.

Thanks laanwj for the review!

It's interesting to see other results, locally this are my results:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      132,289,908.40 |                7.56 |    0.3% |      7.27 | `WalletAvailableCoinsMulti`
|      104,691,758.40 |                9.55 |    0.1% |      5.77 | `WalletAvailableCoinsOnlyBech32`
|      196,688,875.00 |                5.08 |    0.2% |     10.82 | `WalletAvailableCoinsOnlyBech32M`
|       70,386,791.80 |               14.21 |    0.2% |      3.88 | `WalletAvailableCoinsOnlyLegacy`
|      111,477,558.20 |                8.97 |    0.1% |      6.14 | `WalletAvailableCoinsOnlyP2SH_SEGWIT`

But let me do something, going to decrease the bench number of iterations from 5 to 2. It should decrease the total time by half.

Done, decreased the number of iterations to 2. And this are the results:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      129,406,083.50 |                7.73 |    0.0% |      2.85 | `WalletAvailableCoinsMulti`
|      105,704,646.00 |                9.46 |    0.2% |      2.33 | `WalletAvailableCoinsOnlyBech32`
|      206,642,667.00 |                4.84 |    0.6% |      4.55 | `WalletAvailableCoinsOnlyBech32M`
|       69,838,895.50 |               14.32 |    0.0% |      1.54 | `WalletAvailableCoinsOnlyLegacy`
|      112,263,500.00 |                8.91 |    0.0% |      2.47 | `WalletAvailableCoinsOnlyP2SH_SEGWIT`

Total time: ~15 seconds to run the 5 new benchmarks at most here now. Previously, with 5 iterations, total time was ~40 seconds. So, you should see more or less a 40% total time decrease there as well.

Aside from that, I think that a good future question to ask ourselves would be whether we want to always run all the benchmarks all the time or not. Because, sooner or later, as the project isn't going to stop growing, we could end up sacrificing accuracy, implementing not entirely real scenarios (skipping parts of the flow that is being benchmarked) in favor of decreasing the total bench time (there is where my comment in #24924#pullrequestreview-985000006 heads to for example).

But well, this is just me thinking out loud.. after #24924, the total bench time will be pretty decent to not have to worry about this for a while.

Note: haven't updated this PR because a similar benchmark is introduced in #25685.

So, once/if that one gets merged, will rebase this work on top. The amount of changes here will be pretty small.

Approach ACK fd583c8184ef2a270e7df13360f78fd9dda258a0. @furszy are you planning on rebasing this one, now that #25685 was merged ?

Aside from that, I think that a good future question to ask ourselves would be whether we want to always run all the benchmarks all the time or not. Because, sooner or later, as the project isn't going to stop growing, we could end up sacrificing accuracy, implementing not entirely real scenarios...

I can see you also introduced that idea and the improvement is already merged and available; good stuff!

Code review ACK. I've verified most changes were already merged on #25685, waiting for your next commit to re ACK this PR again.

yeah, will rebase + rearrange it in the coming days. Thanks for the ping 👍🏼 .

ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81

ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81

This is my output :

|               ns/op |                op/s |    err% |          ins/op |         bra/op |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:----------
|       10,914,205.50 |               91.62 |    0.1% |  136,775,849.50 |  12,819,423.00 |    0.0% |      0.24 | `WalletAvailableCoins`

tested ACK.

Here my results:

|--------------------:|--------------------:|--------:|----------:|:----------
|       20,730,690.00 |               48.24 |    3.8% |      0.47 | `WalletAvailableCoins`

From time to time I get recommendations to increase the iters as it warns unstability:

|--------------------:|--------------------:|--------:|----------:|:----------
|       20,150,356.50 |               49.63 |    5.3% |      0.48 | :wavy_dash: `WalletAvailableCoins` (Unstable with ~2.0 iters. Increase `minEpochIterations` to e.g. 20)

Not sure if this could be an issue.

ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81