Merge bitcoin/bitcoin#24924: bench: Make WalletLoading benchmark run faster

e673d8b475 bench: Enable loading benchmarks depending on what's compiled (Andrew Chow)
4af3547eba bench: Use mock wallet database for wallet loading benchmark (Andrew Chow)
49910f255f sqlite: Use in-memory db instead of temp for mockdb (Andrew Chow)
a1080802f8 walletdb: Create a mock database of specific type (Andrew Chow)
7c0d34476d bench: reduce the number of txs in wallet for wallet loading bench (Andrew Chow)
f85b54ed27 bench: Add transactions directly instead of mining blocks (Andrew Chow)
d94244c4bf bench: reduce number of epochs for wallet loading benchmark (Andrew Chow)
817c051364 bench: use unsafesqlitesync in wallet loading benchmark (Andrew Chow)
9e404a9831 bench: Remove minEpochIterations from wallet loading benchmark (Andrew Chow)

Pull request description:

  `minEpochIterations` is probably unnecessary to set, so removing it makes the runtime much faster.

ACKs for top commit:
  Rspigler:
    tACK e673d8b475
  furszy:
    Code review ACK e673d8b4, nice PR.
  glozow:
    Concept ACK e673d8b475. For each commit, verified that there was a performance improvement without negating the purpose of the bench, and made some effort to verify that the code is correct.

Tree-SHA512: 9337352ef846cf18642d5c14546c5abc1674b4975adb5dc961a1a276ca91f046b83b7a5e27ea6cd26264b96ae71151e14055579baf36afae7692ef4029800877

src/bench/wallet_loading.cpp

@@ -17,20 +17,19 @@
 #include <optional>
 
 using wallet::CWallet;
+using wallet::DatabaseFormat;
 using wallet::DatabaseOptions;
-using wallet::DatabaseStatus;
 using wallet::ISMINE_SPENDABLE;
 using wallet::MakeWalletDatabase;
+using wallet::TxStateInactive;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 using wallet::WalletContext;
+using wallet::WalletDatabase;
 
-static const std::shared_ptr<CWallet> BenchLoadWallet(WalletContext& context, DatabaseOptions& options)
+static const std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, DatabaseOptions& options)
 {
-    DatabaseStatus status;
     bilingual_str error;
     std::vector<bilingual_str> warnings;
-    auto database = MakeWalletDatabase("", options, status, error);
-    assert(database);
     auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings);
     NotifyWalletLoaded(context, wallet);
     if (context.chain) {
@@ -46,9 +45,47 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
     UnloadWallet(std::move(wallet));
 }
 
+static void AddTx(CWallet& wallet)
+{
+    bilingual_str error;
+    CTxDestination dest;
+    wallet.GetNewDestination(OutputType::BECH32, "", dest, error);
+
+    CMutableTransaction mtx;
+    mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
+    mtx.vin.push_back(CTxIn());
+
+    wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});
+}
+
+static std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options)
+{
+    auto new_database = CreateMockWalletDatabase(options);
+
+    // Get a cursor to the original database
+    auto batch = database.MakeBatch();
+    batch->StartCursor();
+
+    // Get a batch for the new database
+    auto new_batch = new_database->MakeBatch();
+
+    // Read all records from the original database and write them to the new one
+    while (true) {
+        CDataStream key(SER_DISK, CLIENT_VERSION);
+        CDataStream value(SER_DISK, CLIENT_VERSION);
+        bool complete;
+        batch->ReadAtCursor(key, value, complete);
+        if (complete) break;
+        new_batch->Write(key, value);
+    }
+
+    return new_database;
+}
+
 static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
 {
     const auto test_setup = MakeNoLogFileContext<TestingSetup>();
+    test_setup->m_args.ForceSetArg("-unsafesqlitesync", "1");
 
     WalletContext context;
     context.args = &test_setup->m_args;
@@ -57,27 +94,40 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
     // Setup the wallet
     // Loading the wallet will also create it
     DatabaseOptions options;
-    if (!legacy_wallet) options.create_flags = WALLET_FLAG_DESCRIPTORS;
-    auto wallet = BenchLoadWallet(context, options);
+    if (legacy_wallet) {
+        options.require_format = DatabaseFormat::BERKELEY;
+    } else {
+        options.create_flags = WALLET_FLAG_DESCRIPTORS;
+        options.require_format = DatabaseFormat::SQLITE;
+    }
+    auto database = CreateMockWalletDatabase(options);
+    auto wallet = BenchLoadWallet(std::move(database), context, options);
 
     // Generate a bunch of transactions and addresses to put into the wallet
-    for (int i = 0; i < 5000; ++i) {
-        generatetoaddress(test_setup->m_node, getnewaddress(*wallet));
+    for (int i = 0; i < 1000; ++i) {
+        AddTx(*wallet);
     }
 
+    database = DuplicateMockDatabase(wallet->GetDatabase(), options);
+
     // reload the wallet for the actual benchmark
     BenchUnloadWallet(std::move(wallet));
 
-    bench.minEpochIterations(10).run([&] {
-        wallet = BenchLoadWallet(context, options);
+    bench.epochs(5).run([&] {
+        wallet = BenchLoadWallet(std::move(database), context, options);
 
         // Cleanup
+        database = DuplicateMockDatabase(wallet->GetDatabase(), options);
         BenchUnloadWallet(std::move(wallet));
     });
 }
 
+#ifdef USE_BDB
 static void WalletLoadingLegacy(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/true); }
-static void WalletLoadingDescriptors(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/false); }
-
 BENCHMARK(WalletLoadingLegacy);
+#endif
+
+#ifdef USE_SQLITE
+static void WalletLoadingDescriptors(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/false); }
 BENCHMARK(WalletLoadingDescriptors);
+#endif

src/wallet/walletdb.cpp

@@ -1186,13 +1186,36 @@ std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase()
 }
 
 /** Return object for accessing temporary in-memory database. */
+std::unique_ptr<WalletDatabase> CreateMockWalletDatabase(DatabaseOptions& options)
+{
+
+    std::optional<DatabaseFormat> format;
+    if (options.require_format) format = options.require_format;
+    if (!format) {
+#ifdef USE_BDB
+        format = DatabaseFormat::BERKELEY;
+#endif
+#ifdef USE_SQLITE
+        format = DatabaseFormat::SQLITE;
+#endif
+    }
+
+    if (format == DatabaseFormat::SQLITE) {
+#ifdef USE_SQLITE
+        return std::make_unique<SQLiteDatabase>(":memory:", "", options, true);
+#endif
+        assert(false);
+    }
+
+#ifdef USE_BDB
+    return std::make_unique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), "", options);
+#endif
+    assert(false);
+}
+
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase()
 {
     DatabaseOptions options;
-#ifdef USE_SQLITE
-    return std::make_unique<SQLiteDatabase>("", "", options, true);
-#elif USE_BDB
-    return std::make_unique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), "", options);
-#endif
+    return CreateMockWalletDatabase(options);
 }
 } // namespace wallet

src/wallet/walletdb.h

@@ -301,6 +301,7 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, st
 std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase();
 
 /** Return object for accessing temporary in-memory database. */
+std::unique_ptr<WalletDatabase> CreateMockWalletDatabase(DatabaseOptions& options);
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase();
 } // namespace wallet