Use a single wallet batch for UpgradeKeyMetadata #15433
Conversation
jonasschnelli
force-pushed
the
2019/02/wallet_key_upgrade_batch
branch
from
204ad5c to
0bedcba
Compare
|
Tagged for 0.18 because I'd consider this a bugfix |
|
Concept ACK. We should also consider doing that for |
|
utACK and concept ACK 0bedcba |
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
Concept ACK.
On line 387, before throwing, shouldn't it commit the batch?
|
@promag Wouldn't it doe that, as the unique ptr gets destroyed (and flush on exit was set to |
|
@MarcoFalke yap, it wasn't clear. Maybe worth a comment. @jonasschnelli is there a rationale for 1000? |
|
utACK 0bedcba |
As @MarcoFalke said, if the unique pointer gets out of the scope, it "commits" (flushes) to the database.
Not really. I thought 1000 is a good compromise between cache memory requirements and performance. A new constant looked after an overkill. Initially I wanted to tie it to the keypool size constant but somehow I thought then that this has nothing to do with memory consumption. |
|
1000 seems like a good upper bound, since the speedup flattens out after this: (time in us)
|
Summary: This is a backport of [[bitcoin/bitcoin#15433 | PR15433]] Test Plan: ninja all check-all Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D6383
7 participants
Opening wallets (the first time) after #14021 took on my end around 30 seconds due to the keymetadata migration (tested on regtest).
Using a single wallet batch reduces the required time for the migration down to <1s on my system for a default 2k keypool wallet.