[Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) #4805

1. 
   cozz commented at 11:19 pm on August 31, 2014: contributor

   We are currently flushing the wallet after every operation. This is very slow. In case of syncing txs, we need performance, rather than durability. Our SetBestChain-mechanism ensures, that txs from a block are always synced, even on a crash, as we rescan the necessary blocks on startup. The FlushWalletThread only flushes, if there is no wallet-update for 2 seconds.

   I tested replacing the txn_checkpoint(..) call with log_flush, but this does not help.

   Currently I can only add about 10 txs per second to the wallet on my machine. After this patch its more than 1000 txs per second.

   For example importaddress mvqaxopu9Uef2qjXSV3fGrp6DcEYnCoYWK adds about 20000 txs to the wallet. Together with these optimizations #4702 #4712 you hardly notice the difference between a normal rescan and adding 20000 txs to the wallet. Currently this takes hours, I had to let my client running over night, in order to finish. The overall performance increase is like factor 500 or so on a larger wallet.

2. laanwj added the label GUI on Sep 1, 2014
3. laanwj removed the label GUI on Sep 1, 2014
4. laanwj added the label Wallet on Sep 1, 2014
5. in src/wallet.cpp: in 0290d9cfab outdated

   648                 wtx.SetMerkleBranch(pblock);
   649-            return AddToWallet(wtx);
   650+
   651+            // Do not flush the wallet here for performance reasons
   652+            // this is safe, as in case of a crash, we rescan the necessary blocks on startup through our SetBestChain-mechanism
   653+            CWalletDB walletdb(strWalletFile);
   

   ---

   ---

   

   laanwj commented at 9:15 am on September 8, 2014:

   Maybe make fFlushOnClose a constructor argument?
6. in src/wallet.cpp: in 0290d9cfab outdated

   875@@ -869,8 +876,11 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, int64_t& nReceived,
   876 }
   877 
   878 
   879-bool CWalletTx::WriteToDisk()
   880+bool CWalletTx::WriteToDisk(CWalletDB *pwalletdb)
   881 {
   882+    if (pwalletdb)
   

   ---

   ---

   

   laanwj commented at 9:16 am on September 8, 2014:

   I don’t like this duplication. Would it be very invasive to require every client of this function to pass a valid CWalletDB*?
7. 
   laanwj commented at 9:18 am on September 8, 2014: member

   ACK on concept. It is very important to flush immediately on a keypool update, but less so when adding a transaction (as this can be corrected) and then especially en-masse during a rescan.
8. cozz force-pushed on Sep 9, 2014
9. 
   BitcoinPullTester commented at 2:08 am on September 9, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4805_04dbecac9dcc1cc0b8ff450d48104bc09f32d3ab/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
10. 
    cozz commented at 2:14 am on September 9, 2014: contributor

    Addressed inline comments.
11. 
    sipa commented at 2:48 am on October 3, 2014: member

    Needs rebase.
12. [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) 44bc988e7b
13. cozz force-pushed on Oct 3, 2014
14. 
    cozz commented at 3:47 am on October 3, 2014: contributor

    rebased
15. 
    jgarzik commented at 11:57 am on December 31, 2014: contributor

    concept ACK. Makes me dislike our RAII CDB method of I/O for the wallet even more ;p Pondering an alternate way of accomplishing the same thing.

    Gave this an “it works” tests.

16. 
    laanwj commented at 11:14 am on January 26, 2015: member

    The best “alternate solution” would be to get rid of berkeleydb completely (eg #5686).
17. laanwj merged this on Jan 26, 2015
18. laanwj closed this on Jan 26, 2015

    ---

19. laanwj referenced this in commit 8204e19abe on Jan 26, 2015
20. Warrows referenced this in commit 13c6947505 on Jul 19, 2019
21. Warrows referenced this in commit 7cb5f0cca0 on Jul 19, 2019
22. Warrows referenced this in commit 4e8d0f3a05 on Jul 20, 2019
23. Warrows referenced this in commit 6cc4d37ba3 on Jul 28, 2019
24. Warrows referenced this in commit c13bbad9c7 on Aug 4, 2019
25. Warrows referenced this in commit 276fea3936 on Sep 11, 2019
26. Warrows referenced this in commit 7a916a9bd5 on Sep 23, 2019
27. Warrows referenced this in commit 7ccb2b5856 on Oct 9, 2019
28. random-zebra referenced this in commit 0f1764a3db on Oct 9, 2019
29. CaveSpectre11 referenced this in commit 7b2ead60eb on Dec 14, 2019
30. wqking referenced this in commit 0f941285d1 on May 24, 2020
31. Kokary referenced this in commit ff4ec5bbb5 on May 26, 2020
32. Kokary referenced this in commit 3772f6f8cb on Nov 13, 2020
33. Kokary referenced this in commit a65b317c48 on Nov 17, 2020
34. Kokary referenced this in commit a358c40784 on Nov 17, 2020
35. KolbyML referenced this in commit c6a4e7eed0 on Nov 21, 2020
36. Kokary referenced this in commit cdafe64d9a on Nov 24, 2020
37. Cryptarchist referenced this in commit af6613a0a6 on Nov 30, 2020
38. lyricidal referenced this in commit 57440536fa on Jul 24, 2021
39. MarcoFalke locked this on Sep 8, 2021

Contributors
cozz laanwj BitcoinPullTester sipa jgarzik

Labels
Wallet