Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Startup][GUI][Performance] Optimizations for huge wallets. #1217

Merged
merged 4 commits into from
Jan 2, 2020
Merged

[Startup][GUI][Performance] Optimizations for huge wallets. #1217

merged 4 commits into from
Jan 2, 2020

Conversation

furszy
Copy link

@furszy furszy commented Dec 19, 2019

Made several changes in order to improve the wallet startup and navigation performance in huge wallets (wallets with more than 20,000 transactions).
 

  1. Maximum amount of loaded transaction records in the GUI set to 20,000.
  • If the wallet has more, then it will not load them into the UI anymore. Hard to think that any user will be benefited from having 100k txs that he/she made in 2 years always visible when them are most likely all spent and only the latest 5k txs have utxo..
  1. Optimized every listview load in the GUI.
  • The list items are laid out in batches of a certain size, instead of all of the items loaded at once in the first run.
  • Uniform row size flag enabled in every list optimizing their painting.
  1. Db verification decreased to 10 blocks.
  • Previously our wallet was checking, disconnecting and reconnecting the last 100 blocks in the startup, looking for inconsistencies. That is really too much, if we have any inconsistency, it should appear in the last 10 blocks.

@furszy
[Model] Maximum amount of loaded record in ram, preventing any self-s…
b1aa1e2 
…pamming.

Not useful for any user to load into the UI more than 20,000 transaction records, if the wallet has them. Only will make the whole GUI slower and screw the whole user experience. This sets a limit and only shows the latest 20k txs (which is most likely still a big number for any regular user..).
@furszy
[GUI][Performance] Optimizations to every list view load in the GUI.
7c9dd0d 
Batching items load + uniform item sizes.
@furszy
[Startup] Decrease the amount of blocks checked for corruption in the…
6d5c84e 
… startup.

100 blocks checked for corruption is too much, having only 10 is more than enough to verify the db consistency.
@furszy furszy self-assigned this Dec 19, 2019
@Fuzzbawls
Copy link
Collaborator

Been running this for a bit on a fairly large testnet wallet and there seems to be some noticeable improvement in responsiveness.

@furszy furszy changed the title [WIP][Startup][GUI][Performance] Optimizations for huge wallets. [Startup][GUI][Performance] Optimizations for huge wallets. Dec 22, 2019
random-zebra
random-zebra reviewed Dec 23, 2019
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty good changes.

src/qt/transactiontablemodel.cpp Outdated Show resolved Hide resolved
@random-zebra random-zebra added Data Corruption GUI Startup Wallet startup changes and/or improvements labels Dec 23, 2019
@random-zebra random-zebra added this to the 4.1.0 milestone Dec 23, 2019
@furszy furszy mentioned this pull request Dec 28, 2019
Merged
@random-zebra random-zebra modified the milestones: 4.1.0, 4.0.1 Dec 29, 2019
Fuzzbawls
Fuzzbawls approved these changes Jan 1, 2020
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7a1a639

random-zebra added a commit that referenced this pull request Jan 2, 2020
@random-zebra
Merge #1231: [Wallet] IsEquivalentTo commented, removing an extra rou…
ccb2402 
…nd of ser+hash calculation.

24fb46d [Wallet] IsEquivalentTo commented for now to not add an extra round of serialization + hash calculation in every read transaction in the wallet startup. (furszy)

Pull request description:

  To explain the reason that forced me to hunt this for over a week..

  In huge wallets (+100k txs wallets) the 4.0 startup time is taking +30 minutes. Which is crazy long and sounded much more as a regression over 3.4 which was taking less than 10 minutes.

  Root cause:

  After some time (and improvements over different areas #1217), found that the `IsEquivalentTo` method is adding an extra round of serialization + hash calculation on every transaction that is loaded into the wallet. Which gets really, time-wise, expensive in big wallets for a small benefit.

  Test:

  Environment:
  - Wallet with 419,633 db records.
  - MacOS i7, 16gb ram.

  Pre-PR transactions load time:

  30 minutes and i got bored and killed the process..

  Post-PR transactions load time:

  34.369 seconds.

  For now, decided to just comment it to be able to release 4.0.1 as soon as possible with all of the improvements.

  The final, future, goal of course it's not this one, we need to backport the whole `loadToWallet` flow from upstream + introduce our own changes there.

  Just a first initial step solving the regression.

ACKs for top commit:
  Fuzzbawls:
    ACK 24fb46d
  random-zebra:
    ACK 24fb46d and merging...

Tree-SHA512: bd4b7b34b60eabf9ebe555312bbb26373430057bff07d1ee1d0bc137004dd47bfbf3d263e146113e347b3a767b4e01376fdf3b2984abd4b321d188ac91cfa391
@random-zebra random-zebra mentioned this pull request Jan 2, 2020
Open
random-zebra
random-zebra approved these changes Jan 2, 2020
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7a1a639 and merging...

random-zebra added a commit that referenced this pull request Jan 2, 2020
@random-zebra
Merge #1217: [Startup][GUI][Performance] Optimizations for huge wallets.
d49e4a6 
7a1a639 [Trivial] Adding load txs on demand todo text. (furszy)
6d5c84e [Startup] Decrease the amount of blocks checked for corruption in the startup. (furszy)
7c9dd0d [GUI][Performance] Optimizations to every list view load in the GUI. (furszy)
b1aa1e2 [Model] Maximum amount of loaded record in ram, preventing any self-spamming. (furszy)

Pull request description:

  Made several changes in order to improve the wallet startup and navigation performance in huge wallets (wallets with more than 20,000 transactions).
   

  1) Maximum amount of loaded transaction records in the GUI set to 20,000.
  * If the wallet has more, then it will not load them into the UI anymore. Hard to think that any user will be benefited from having 100k txs that he/she made in 2 years always visible when them are most likely all spent and only the latest 5k txs have utxo..

  2) Optimized every listview load in the GUI.
  * The list items are laid out in batches of a certain size, instead of all of the items loaded at once in the first run.
  * Uniform row size flag enabled in every list optimizing their painting.

  3) Db verification decreased to 10 blocks.
  * Previously our wallet was checking, disconnecting and reconnecting the last 100 blocks in the startup, looking for inconsistencies. That is really too much, if we have any inconsistency, it should appear in the last 10 blocks.

ACKs for top commit:
  Fuzzbawls:
    ACK 7a1a639
  random-zebra:
    ACK 7a1a639 and merging...

Tree-SHA512: 66f2241d881bd82a97848ad6c3975b7b512cde9bf6d42a6c67c2864cf7382e40ce99e611085f673abaff604decd9fd00acdd75a63211e1c89c82ce2cc0fc0bef
@random-zebra random-zebra merged commit 7a1a639 into PIVX-Project:master Jan 2, 2020
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jan 11, 2020
@furszy @Fuzzbawls
[Model] Maximum amount of loaded record in ram, preventing any self-s…
7da7eb7 
…pamming.

Not useful for any user to load into the UI more than 20,000 transaction records, if the wallet has them. Only will make the whole GUI slower and screw the whole user experience. This sets a limit and only shows the latest 20k txs (which is most likely still a big number for any regular user..).

Github-Pull: PIVX-Project#1217
Rebased-From: b1aa1e2
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jan 11, 2020
@furszy @Fuzzbawls
[GUI][Performance] Optimizations to every list view load in the GUI.
3b67bc6 
Batching items load + uniform item sizes.

Github-Pull: PIVX-Project#1217
Rebased-From: 7c9dd0d
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jan 11, 2020
@furszy @Fuzzbawls
[Startup] Decrease the amount of blocks checked for corruption in the…
784094b 
… startup.

100 blocks checked for corruption is too much, having only 10 is more than enough to verify the db consistency.

Github-Pull: PIVX-Project#1217
Rebased-From: 6d5c84e
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jan 11, 2020
@furszy @Fuzzbawls
[Trivial] Adding load txs on demand todo text.
ee4aed3 
Github-Pull: PIVX-Project#1217
Rebased-From: 7a1a639
@Fuzzbawls Fuzzbawls mentioned this pull request Jan 11, 2020
Merged
Fuzzbawls added a commit that referenced this pull request Jan 12, 2020
@Fuzzbawls
Merge #1255: [Backport] 4.0.1 backports
38d641d 
d209897 [RPC] Notate all account stuff as deprecated (Fuzzbawls)
10bbf9e [Qt] Initialize isLoading to false for CS view (Fuzzbawls)
b7c63b8 [Trivial] Set log2_work decimals to 16 in the logs Github-Pull: #1252 Rebased-From: d83e67c (random-zebra)
a80e2e9 [RPC] Fix getstakingstatus removing compile-time conditional (random-zebra)
876d393 [Wallet][Cleanup][GUI] minor updates to staking status (random-zebra)
f9f437a [Wallet] CStakerStatus: save a pointer to the tip instead of the hash Github-Pull: #1245 Rebased-From: 968d861 (random-zebra)
f9022cf [GUI] Refactor updateStakingStatus and set it to inactive after locking (random-zebra)
cd6f6cb [Trivial] Fix lastHashTime type in miner Github-Pull: #1245 Rebased-From: d2aebc5 (random-zebra)
6845d18 [RPC] Add CStakerStatus data to getstakingstatus Github-Pull: #1245 Rebased-From: b8ed76f (random-zebra)
b63d963 [PoS] Lock cs_main when getting chainActive data in miner Github-Pull: #1245 Rebased-From: d2d5f08 (random-zebra)
6fac9b9 [Cleanup] Remove unused variables in miner (random-zebra)
7128945 [Core][PoS] Replace mapHashedBlocks and nLastCoinStakeSearchInterval (random-zebra)
c546ac1 [Cleanup][Wallet] Remove unused nSearchInterval field in CreateCoinStake Github-Pull: #1245 Rebased-From: 6b2b813 (random-zebra)
5dbda15 [GUI] Every masternode action checking for tier two network synced. (furszy)
499dac8 [Trivial][RPC] Fix example line in importprivkey help (missing arg) Github-Pull: #1242 Rebased-From: ce93872 (random-zebra)
4133401 [Tests] Add wallet_import_stakingaddress to test runner (random-zebra)
8ac5cbb [Tests] Add functional test for import staking address/key Github-Pull: #1242 Rebased-From: d1ebf2e (random-zebra)
bb9426a [RPC] Add coldstaking address support in importaddress Github-Pull: #1242 Rebased-From: 3bd5579 (random-zebra)
3084143 [RPC] Add coldstaking address support in importprivkey Github-Pull: #1242 Rebased-From: 144ec35 (random-zebra)
bb021af [Model][Wallet][Performance] TxRecord updateStatus not accepted stake status fix + performance improvements. (furszy)
1f0823f [GUI][Model] Start missing masternodes flow implemented. (furszy)
90639d1 [GUI] Masternodes start all flow implemented. (furszy)
14f3220 [Core] Don't log missing MNs during CBudgetProposal::CleanAndRemove (random-zebra)
7e1b908 [GUI] Translator class abstracted to be able to reuse the ProcessSendCoinsReturn method in the masternodeswizard class. (furszy)
fa73ae3 [GUI] Masternode creation wizard, creation fail event properly detailing the failure cause in the subsequent error dialog. (furszy)
447265f [Wallet] Graceful shutdown in the unlock corrupted wallet,  showing the proper error message in screen. (furszy)
81cbe1d [Cleanup][Tests] Remove precompute option in default framework node conf Github-Pull: #1237 Rebased-From: 51cbb00 (random-zebra)
ded2cc2 [Cleanup] remove fPrecompute option in SelectStakeCoins (random-zebra)
391c9e6 [Cleanup][Tests] Remove crazy useful unittest created by "Tom" Github-Pull: #1237 Rebased-From: 0187f0d (random-zebra)
67411b5 [Cleanup][Wallet] Remove zpiv spend cache from zpiv tracker Github-Pull: #1237 Rebased-From: 29a934c (random-zebra)
7084840 [Cleanup][DB] Remove DB functions for zerocoin precomputing Github-Pull: #1237 Rebased-From: bd3eebb (random-zebra)
1e93feb [Cleanup] Remove zPIV precomputing global variables. Github-Pull: #1237 Rebased-From: 837f4e2 (random-zebra)
4de0dec [Cleanup][Qt] remove COLUMN_PRECOMPUTE from zpivcontroldialog Github-Pull: #1237 Rebased-From: 520284e (random-zebra)
3394078 [Cleanup] Init: remove precompute-related helps in strUsage Github-Pull: #1237 Rebased-From: 2585532 (random-zebra)
7d03510 [Cleanup] Init: remove "precompute" debug category (not used anywhere) Github-Pull: #1237 Rebased-From: fbcf37b (random-zebra)
9ea6aa2 [Tests][BUG] Fix RPC_TRANSACTION_REJECTED in mining_pos_reorg Github-Pull: #1218 Rebased-From: 1fb9dd3 (random-zebra)
44c60dc [Tests] sort exported mints by denom in zc_spends and zc_wrapped_serials (random-zebra)
0538d25 [Tests] Speed up cache generation (random-zebra)
3abe8a8 [Travis][Tests] Add travis ping during create_cache Github-Pull: #1218 Rebased-From: 6a2a3b9 (random-zebra)
e6080f4 [RPC][BUG] Remove extra lock in spendrawzerocoin Github-Pull: #1218 Rebased-From: 35ea5bc (random-zebra)
88ddf09 [Tests][Trivial] remove stale zerocoin_publicSpend_reorg (random-zebra)
5c70582 [Tests] Fix and re-enable wrapping serials test Github-Pull: #1218 Rebased-From: 039c220 (random-zebra)
fc7069a [RPC] Enable v2 spending on regtest with spendrawzerocoin Github-Pull: #1218 Rebased-From: ddf18d9 (random-zebra)
5a3951a [Tests] Sort test_runner list by execution time (and comment it there) Github-Pull: #1218 Rebased-From: b3af5bd (random-zebra)
248adc5 [Tests] Fix edge case for double spends in zerocoin_spends.py (random-zebra)
66f3cb1 [Tests] Remove zpiv tests from fakestake (random-zebra)
11bc853 [Trivial] Rename base test class in p2p_time_offset functional test Github-Pull: #1218 Rebased-From: cb54381 (random-zebra)
8a25781 [Tests] Refactor POS reorg test (random-zebra)
ddc6b8a [Tests] Prefix "_pos_" tests with "_mining_" Github-Pull: #1218 Rebased-From: 384c636 (random-zebra)
97da652 [Tests] Fix wallet_reorg-stake functional test Github-Pull: #1218 Rebased-From: 8949e38 (random-zebra)
807b4ab [Consensus] Fix fake-stake spent serials detection on forked chains Github-Pull: #1218 Rebased-From: 0417f66 (random-zebra)
24bee71 [Tests] Unify and complete FakeStake test suite in a single testcase. Github-Pull: #1218 Rebased-From: d2c1ef2 (random-zebra)
0dc9e17 [Tests] Refactor PIVX tools in test_framework, blocktools and utils Github-Pull: #1218 Rebased-From: b3f0c59 (random-zebra)
07a7081 [RPC][Tests] createrawzerocoinpublicspend --> createrawzerocoinspend (random-zebra)
ccd5b80 [Tests] Refactor zerocoin_valid_public_spend into zerocoin_spends (random-zebra)
0b551a9 [Tests] Add zerocoins to PoS cache (random-zebra)
ae841ed [RPC] use json object as output of mintzerocoin (random-zebra)
5539b01 [Tests] Restore p2p_zpos_fakestake (random-zebra)
8f84377 [Tests] Lower zerocoin confirmations on regtest (random-zebra)
f36a035 [Tests] Fix zerocoin_valid_public_spend with new v3 activation height Github-Pull: #1218 Rebased-From: acf8fea (random-zebra)
d7804b7 [Tests] Set public spend activation for regtest to block 400 (from 350) (random-zebra)
e8b7ce3 [RPC] Refactor spendzerocoin/spendzcoinmints, fix createrawzerocoinstake (random-zebra)
9134f40 [Tests] Introduce PIVX specific blocktools (random-zebra)
56cfbdd [Tests] Cleanup rpc_spork functional test with new util functions Github-Pull: #1218 Rebased-From: 68ce4e9 (random-zebra)
e5e6f28 [Tests] Define spork util functions (random-zebra)
ac6f95e [Tests] Use PoS_cache in zerocoin_public_spend Github-Pull: #1218 Rebased-From: 383ab89 (random-zebra)
e093f7b [RPC] Add mint txid to spendrawzerocoin (and look for it if empty) Github-Pull: #1218 Rebased-From: 422f003 (random-zebra)
8ebd65a [Trivial] Detailed debug log for received spork messages Github-Pull: #1218 Rebased-From: afabe8c (random-zebra)
f669844 [Tests] Define a new 'PoS' cache with 330-blocks chain Github-Pull: #1218 Rebased-From: 80306e1 (random-zebra)
3c84efd [Tests] Fix CheckBlockHeader version after block 300 for regtest Github-Pull: #1218 Rebased-From: af05235 (random-zebra)
3b8de3a [Tests] Have combine_logs default to most recent test dir (random-zebra)
baa97af [Tests] rename base class to PivxTestFramework Github-Pull: #1218 Rebased-From: 8f668b6 (random-zebra)
ab553bf [Tests] Debug assert_raises_rpc_error Github-Pull: #1218 Rebased-From: 261bb6a (random-zebra)
76531e2 [Cleanup] Replace addr purpose strings with AddressBookPurpose constants Github-Pull: #1238 Rebased-From: 768694f (random-zebra)
09120ce [RPC] Fix Base58Type in ListaddressesForPurpose (random-zebra)
7e32b1e [RPC] Add optional bool arg to listdelegators to show blacklisted addrs Github-Pull: #1238 Rebased-From: 34e871c (random-zebra)
8f01bce [RPC] Add optional str argument to delegatoradd for address label Github-Pull: #1238 Rebased-From: 21f1b9d (random-zebra)
bb1c8f2 [Docs] Update build-unix.md (Fuzzbawls)
f035a4a [Cleanup] Remove precomputing (Fuzzbawls)
ee4aed3 [Trivial] Adding load txs on demand todo text. (furszy)
784094b [Startup] Decrease the amount of blocks checked for corruption in the startup. (furszy)
3b67bc6 [GUI][Performance] Optimizations to every list view load in the GUI. (furszy)
7da7eb7 [Model] Maximum amount of loaded record in ram, preventing any self-spamming. (furszy)
aeaf418 [GUI] Adding the capability to decrease the screen size for smaller screens support. (furszy)
2a5d14d [Wallet] IsEquivalentTo commented for now to not add an extra round of serialization + hash calculation in every read transaction in the wallet startup. (furszy)
f7a1420 Minor changes within 4.0 wallet FAQ Github-Pull: #1224 Rebased-From: 43b783f (NoobieDev12)
053d365 [Trivial] Remove spammy log in in StakeV1 Github-Pull: #1233 Rebased-From: f9857f7 (random-zebra)
16441e9 [GUI][Trivial] Minor edits to written content Github-Pull: #1184 Rebased-From: dae64f3 (random-zebra)
ffac2a7 [BUG][RPC] fix signature check (against old format) in mnbudgetrawvote Github-Pull: #1206 Rebased-From: ae063d0 (random-zebra)
e82e2f5 [GUI][Bug] Show locked balance in the available total amount and notify the user about its existence. (furszy)
47d2c40 [Backport][Performance] Cache best block hash for miner thread usage + refactor. (furszy)
fe12794 [Wallet] Remove un-necessary CheckTransaction call when loading wallet. (Fuzzbawls)
b8f2b1a [Build] Clean up warnings (Cave Spectre)
d4e6693 [Cleanup] Remove unnecessary QtCreator files (Fuzzbawls)
361757b [Build] Bump snapcraft nightlies to 4.0.99 (Fuzzbawls)

Pull request description:

  Updates the `4.0` branch with relevant merged PRs in preparation for tagging the `4.0.1` release.

  Included PRs:
  #1204
  #1205
  #1199
  #1222
  #1203
  #1223
  #1206
  #1184
  #1233
  #1224
  #1231
  #1228
  #1217
  #1234
  #1207
  #1238
  #1218
  #1237
  #1229
  #1211
  #1243
  #1221
  #1240
  #1242
  #1250
  #1245
  #1252
  #1253
  #1251

ACKs for top commit:
  random-zebra:
    utACK d209897

Tree-SHA512: 092349a93ea4bfe6d6284d5b996055d2900576fab971707458260a848fbdbb15926b13e6ee46469b283721f7dc51a455f5d209a62d9309caa03cecd3b66bd556
@furszy furszy deleted the 2019_optimizations branch November 29, 2022 14:06
@lyricidal lyricidal mentioned this pull request Jun 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@random-zebra random-zebra random-zebra approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@furszy furszy

Labels
Data Corruption GUI Startup Wallet startup changes and/or improvements
Projects
None yet
Milestone
4.0.1
Development

Successfully merging this pull request may close these issues.
3 participants
@furszy @Fuzzbawls @random-zebra