tests: speed up coinselector_tests #23338

1. 
   achow101 commented at 9:21 pm on October 22, 2021: member

   #23288 changed coinselector_tests to use DescriptorScriptPubKeyMan, but it also ended up significantly slowing down the test, from 4 seconds to over 1 minute. It appears that the source of this slow down is with CWallet::AddToWallet, and primarily due to writing data to the mock wallet database. Because the only thing that is actually needed is for the created transaction to be placed into CWallet::mapWallet, this PR removes the call to AddToWallet and just places the transaction into mapWallet directly. This reduces the test time to 5 seconds.

   To speed things up further, CreateMockWalletDatabase is changed to make a SQLiteDatabase instead of a BerkeleyDatabase. This is safe because there are no tests that require a specific mock database type.

2. 
   achow101 commented at 9:21 pm on October 22, 2021: member

   cc @jnewbery
3. tests: Place into mapWallet in coinselector_tests

   Instead of using AddToWallet so that making a COutput will work,
   directly add the transaction into wallet.mapWallet. This bypasses many
   checks that AddToWallet will do which are pointless and just slow down
   this test.

   a78c229808
4. walletdb: Use SQLiteDatabase for mock wallet databases

   Default to SQLiteDatabase instead of BerkeleyDatabase for
   CreateDummyWalletDatabase. Most tests already use descriptor wallets and
   the mock db doesn't really matter for tests. The tests where it does
   matter will make the db directly.

   a52f1d1340
5. achow101 force-pushed on Oct 22, 2021
6. DrahtBot added the label Wallet on Oct 22, 2021
7. brunoerg approved
8. 
   brunoerg commented at 12:05 pm on October 23, 2021: member

   tACK a52f1d13409e4ef46277596ec13fa8b421fa1329

   Ran the test on the master branch, it didn’t take over 1 minute here, took about 30secs, but now it is taking ~3 seconds. (Macbook M1)

9. lsilva01 approved
10. 
    lsilva01 commented at 6:57 am on October 25, 2021: contributor

    tACK a52f1d1. Performed 74.36% better on Ubuntu 20.04 (VM, 12 MB, 8vCPU).
11. 
    glozow commented at 11:24 am on October 25, 2021: member

    utACK a52f1d13409e4ef46277596ec13fa8b421fa1329

    Since it’s a coin selection unit test and add_coin’s job is to populate the coins vector with UTXOs to select from, calling mapWallet.emplace() directly seems sensible. I don’t know this code very well but AFAICT none of the coin selection functions rely on the something done in AddToWallet. On my machine, the test sped up from 33sec to 3sec.

    Side note: the AddToWallet function declaration is missing a doxygen comment. It would be nice to add one.

12. MarcoFalke merged this on Oct 25, 2021
13. MarcoFalke closed this on Oct 25, 2021

    ---

14. sidhujag referenced this in commit 7b4431664b on Oct 25, 2021
15. DrahtBot locked this on Oct 30, 2022

Contributors
achow101 brunoerg lsilva01 glozow

Labels
Wallet