bench: replace CreateMockableWalletDatabase with MakeInMemoryWalletDatabase#35705
Merged
sedited merged 1 commit intoJul 13, 2026
Merged
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35705. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste |
b9ef7b0 to
b0de804
Compare
Contributor
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
…tabase Benchmarks don't need mock-specific behaviour (overridden Filename(), Format(), or the exposed batch-level WriteKey()). Replace CreateMockableWalletDatabase() with MakeInMemoryWalletDatabase() across 6 call sites in src/bench/ (5 files), using the same in-memory SQLite path that production code uses. wallet_migration.cpp is excluded: it calls GetOrCreateLegacyDataSPKM() which asserts Format() == "sqlite-mock", a signal used to allow legacy SPKM setup in test/bench contexts. MockableSQLiteDatabase is still correct there. For coin_selection.cpp, which had no other dependencies on wallet/test/util.h, also switch the include to <wallet/sqlite.h>. Follow-up suggested in bitcoin#35655.
b0de804 to
7508ac3
Compare
sedited
approved these changes
Jul 13, 2026
janb84
reviewed
Jul 13, 2026
janb84
left a comment
Contributor
There was a problem hiding this comment.
ACK 7508ac3
PR removes an unnecessary test-only indirection, after #35655. LGTM
Code review, build and checked if this has any consequences for benchmarking (all within variance)
Details
This PR:
| ns/selection | selection/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 4,188,525.00 | 238.75 | 0.1% | 0.46 | `CoinSelection`
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 35,623,583.00 | 28.07 | 0.0% | 0.39 | `WalletAvailableCoins`
| 3,083.00 | 324,359.39 | 1.3% | 0.00 | `WalletBalanceClean`
| 3,125.00 | 320,000.00 | 0.0% | 0.00 | `WalletBalanceDirty`
| 41.00 | 24,390,243.90 | 2.4% | 0.00 | `WalletBalanceWatch`
| 807,584.00 | 1,238.26 | 0.4% | 0.01 | `WalletCreateTxUseOnlyPresetInputs`
| 21,648,458.00 | 46.19 | 1.8% | 0.24 | `WalletCreateTxUsePresetInputsAndCoinSelection`
| ns/key | key/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 199,289.15 | 5,017.83 | 0.3% | 4.39 | `WalletEncryptDescriptors`
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 37.46 | 26,692,554.06 | 0.2% | 0.01 | `WalletIsMineDescriptors`
| 37.86 | 26,411,598.60 | 0.3% | 0.01 | `WalletIsMineMigratedDescriptors`Master:
| ns/selection | selection/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 4,128,975.00 | 242.19 | 0.2% | 0.45 | `CoinSelection`
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 35,544,333.00 | 28.13 | 0.1% | 0.39 | `WalletAvailableCoins`
| 3,083.00 | 324,359.39 | 1.3% | 0.00 | `WalletBalanceClean`
| 3,166.00 | 315,855.97 | 1.3% | 0.00 | `WalletBalanceDirty`
| 41.00 | 24,390,243.90 | 2.4% | 0.00 | `WalletBalanceWatch`
| 729,834.00 | 1,370.17 | 0.3% | 0.01 | `WalletCreateTxUseOnlyPresetInputs`
| 21,700,541.00 | 46.08 | 1.7% | 0.24 | `WalletCreateTxUsePresetInputsAndCoinSelection`
| ns/key | key/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 198,431.87 | 5,039.51 | 0.2% | 4.37 | `WalletEncryptDescriptors`
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 36.81 | 27,168,938.22 | 0.1% | 0.01 | `WalletIsMineDescriptors`
| 37.38 | 26,750,031.75 | 0.4% | 0.01 | `WalletIsMineMigratedDescriptors`
alexanderwiederin
added a commit
to alexanderwiederin/rust-bitcoinkernel
that referenced
this pull request
Jul 14, 2026
…4e48e968a 114e48e968a kernel: Add script tracer 54e1a95a12e Merge bitcoin/bitcoin#35719: ci: disable Qt build in OpenBSD cross job b0e09511586 ci: disable Qt build in OpenBSD cross job 734c34bafda Merge bitcoin/bitcoin#35427: depends: Build `qt` and `qrencode` packages on OpenBSD ee61b11a9e7 Merge bitcoin/bitcoin#35200: node: smooth oversized `dbcache` warnings e3554bf361f Merge bitcoin/bitcoin#35579: wallet: reserve walletrescan before checking wallet is at the tip 11ae4265522 Merge bitcoin/bitcoin#35715: cmake: Fix WITH_EXTERNAL_LIBMULTIPROCESS + BUILD_FUZZ_BINARY e544413c0de Merge bitcoin/bitcoin#32763: wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members fe1cb6e40d7 Merge bitcoin/bitcoin#35690: wallet: Introduce WalletError with machine-readable error code 441f3114f57 Merge bitcoin/bitcoin#35659: Clarify supported *BSD releases and drop outdated workarounds 0399df827c6 Merge bitcoin/bitcoin#35708: depends: capnp 1.5.0 1ab1fdd4696 Merge bitcoin/bitcoin#35705: bench: replace CreateMockableWalletDatabase with MakeInMemoryWalletDatabase db35b9238fc ipc # build: Fix fuzz target CMakeLists.txt for external libmultiprocess d18fec892e2 Merge bitcoin/bitcoin#35698: doc: Update enum class constant naming style guide a2e4cd7ad2a depends: capnp 1.5.0 7508ac319d9 bench: replace CreateMockableWalletDatabase with MakeInMemoryWalletDatabase 907e284e303 Merge bitcoin/bitcoin#35701: test: Remove `mock_process.cpp` c8459b6bdcd Merge bitcoin/bitcoin#35568: txospenderindex: disable bloom filters to optimize disk usage 63c5f9d22c0 test: Remove `mock_process.cpp` ef101b04a8d Merge bitcoin/bitcoin#35655: wallet: Use in-memory SQLite for temporary wallet in exportwatchonlywallet b6becf3534c Merge bitcoin/bitcoin#35684: Update libmultiprocess subtree to add `max_connections` option 930f25050f7 Merge bitcoin/bitcoin#35700: doc: archive release notes for v29.4 9b2b3f4ec6f doc: archive release notes for v29.4 297fd1489bb Merge bitcoin/bitcoin#35412: ci: add NetBSD Clang cross job e314869066b Merge bitcoin/bitcoin#35695: Remove myself as security contact e3d67a5eae5 Merge bitcoin/bitcoin#35691: chainparams: delete my DNS seed a8223bb4e62 wallet: Introduce WalletError with machine-readable error code fad5809cb92 doc: Update enum class constant naming style guide 7d8137c1417 Merge bitcoin/bitcoin#34897: indexes: Don't commit ahead of the flushed chainstate 629df81e4c6 Remove myself as security contact d164a043426 node: smooth oversized `-dbcache` warnings d9080639804 chainparams: delete my DNS seed e9ed898a0da validation: Don't use m_chain.Tip() in FlushStateToDisk 3679f1ecf5e index: Don't commit ahead of the flushed chainstate 65735728a5a index: Remove return value from Commit() 09c06960c6a validation: track last flushed block 13c02b5466d test: add test for index commits ahead of the last flushed block c43b7a1115f ci: add netBSD cross CI job 699c21aea47 depends: add netbsd_LDFLAGS 777d23f25c7 test: add regression test for in-memory SQLiteDatabase reopen d1e7f8c986f wallet: use in-memory SQLite for temporary wallet in exportwatchonlywallet ee43743f126 wallet: store m_additional_flags in SQLiteDatabase to fix reopen path a9d1b652f32 Merge commit '707d0ded84563386f770ec17970834c65f8fa938' into pr/subtree-12 707d0ded845 Squashed 'src/ipc/libmultiprocess/' changes from 16bf05dea02..28e056576a3 2bab6bc73f2 refactor: Drop support for FreeBSD < 14 91b5c8a07c6 refactor: Remove FreeBSD-specific workaround 56701ff6d5c doc: Clarify supported *BSD releases 6d0ea4cf5bd doc: add release notes a2b1c86903a txospenderindex: disable bloom filters to optimize disk usage fed3cf6f0ed wallet: Replace CWalletTx's vOrderForm with specific fields 4f8823e8e11 wallet: Drop vOrderForm from CommitTransaction 9e62e4b1f34 test: slow down rescaning process a2b0bfcd854 wallet: Drop mapValue from CWalletTx cb99864c91c wallet: Throw if unknown entry is found in mapValue 98d5cdae663 wallet: Make CWalletTx "replaces_txid" and "replaced_by_txid" member variables 7ef8a6efc2a wallet: Make CWalletTx "comment" and "to" member variables 2155e913d3e wallet: Make CWalletTx "from" and "message" member variables c6ba98dcc8a wallet: Drop mapValue from CommitTransaction 00abb174a80 wallet: Pass comment and comment_to to CommitTransaction 1a219a37a21 wallet: Pass replaces_txid to CommitTransaction outside of mapValue 336f5a738b3 wallet: reserve walletrescan before checking wallet is at the tip a54ec373a69 depends: Build `qt` and `qrencode` packages for OpenBSD hosts git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: 114e48e968a087f74c1ab611ac2a31a9266812e1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmarks don't need mock-specific behaviour (overridden
Filename(),Format(), or the exposed batch-levelWriteKey()). ReplaceCreateMockableWalletDatabase()withMakeInMemoryWalletDatabase()across all 6 call sites insrc/bench/(5 files - 4 wallet bench files + 1 in coin_selection), using the same in-memory SQLite path that production code uses.wallet_migration.cppis excluded: it callsGetOrCreateLegacyDataSPKM()which assertsFormat() == "sqlite-mock", a deliberate signal that allows legacy SPKM setup intest/benchcontexts.MockableSQLiteDatabaseis still correct there.For
coin_selection.cpp, which had no other dependencies onwallet/test/util.h, the include is switched to<wallet/sqlite.h>. The remaining 4 files retainwallet/test/util.hfor other utilities but also add an explicit<wallet/sqlite.h>include as required by IWYU.Follow-up suggested in #35655.