[WIP] Wallet: Cache CWalletDB pointer in CWallet to improve performance #6966

pull pstratem wants to merge 1 commits into bitcoin:master from pstratem:wallet_speedup changing 6 files +55 −90

pstratem commented at 4:24 AM on November 7, 2015: contributor

No description provided.
pstratem renamed this:
~~Cache CWalletDB pointer in CWallet to improve performance~~
Wallet: Cache CWalletDB pointer in CWallet to improve performance
on Nov 7, 2015
pstratem renamed this:
~~Wallet: Cache CWalletDB pointer in CWallet to improve performance~~
[WIP] Wallet: Cache CWalletDB pointer in CWallet to improve performance
on Nov 7, 2015
jonasschnelli commented at 7:44 PM on November 7, 2015: contributor

Concept ACK. Travis repots issue with rpc tests. Keeping the DB connection permanently open – I agree – is the way to go. Although it might introduce new bugs.
pstratem commented at 7:29 AM on November 8, 2015: contributor

@jonasschnelli this is very very much a WIP :)
jonasschnelli commented at 8:52 AM on November 8, 2015: contributor

Right. I almost oversaw the WIP tag. Nice work btw!
laanwj commented at 5:32 AM on November 9, 2015: member

Have you measured the performance improvements?

Concept ACK but I'm not convinced that this will not result in less robustness with regard to database flushes (see my other comment). This is essential, even more so for the wallet than for the utxo database.
laanwj added the label Docs and Output on Nov 9, 2015
laanwj added the label Wallet on Nov 9, 2015
laanwj removed the label Docs and Output on Nov 9, 2015

in src/wallet/wallet.cpp:None in 5ffbcb3f9a outdated

 273 | @@ -274,7 +274,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
 274 |                      return false;
 275 |                  if (!crypter.Encrypt(vMasterKey, pMasterKey.second.vchCryptedKey))
 276 |                      return false;
 277 | -                CWalletDB(strWalletFile).WriteMasterKey(pMasterKey.first, pMasterKey.second);

laanwj commented at 5:33 AM on November 9, 2015:

There is a reason we create CWalletDB objects every time per operation. From what I remember this assures that the transactions are closed/flushed properly.

pstratem commented at 5:44 AM on November 9, 2015: contributor

@laanwj I have not specifically but "much much much faster" is how i would describe it
laanwj commented at 5:59 AM on November 9, 2015: member

OK. That's good to hear. Just need to verify that it doesn't come at the expense of robustness, then.
pstratem commented at 10:01 PM on November 18, 2015: contributor

@laanwj CDB::~CDB simply calls CDB::Close which basically just calls Flush

So it's simply a matter of ensuring that we call Flush properly
pstratem commented at 10:02 PM on November 18, 2015: contributor

The easier thing to do is to simple change the default for fFlushOnClose to false, but that doesn't allow for optimizing batch operations such as keypoolrefill (which was actually my original motivation for this change).
Cache CWalletDB pointer in CWallet to improve performance 394f106500
pstratem commented at 12:36 AM on November 21, 2015: contributor

Will do this more incrementally.
pstratem closed this on Nov 21, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels