From 93d3e39010ea1d89c453efa8320bdc8d606dd7f5 Mon Sep 17 00:00:00 2001 From: jamescowens Date: Sun, 19 Jul 2020 02:29:46 -0400 Subject: [PATCH 1/3] Optimize OverviewPage::updateTransactions() For wallets with a very large number of transactions, the filter->setLimit(numItems) is extremely expensive... on the order of a few seconds, because it also resorts the list if sort is defined (and it is). This simply stores the number of rows required for the overview page in the filter, and only calls setLimit if the number of rows is changed with a resize, etc. beyond a reasonable point (3x on purpose). Instead the ui->listTransactions->setRowHidden function is called in a for loop that dynamically sets the rows hidden that are not displayable between the actual display limit and the setLimit value (which is sized initially 3x the number of rows displayable). This eliminates the delay when flipping back and forth between the overview and the send, receive, or transaction views. It also improves the resize of the window. I have picked scaling parameters on the size of getLimit versus the number of rows displayable that should minimize or eliminate GUI freezes under normal window resizing. The wallet still does a list resort every time a block is received, because the underlying wallet model is updated, so that will still cause a GUI freeze for wallets with a large number of transactions. (Note that I am talking about 40000+ transactions.) This cannot be fixed without a major overhaul architecturally. --- src/qt/overviewpage.cpp | 45 +++++++++++++++++++++++++------ src/qt/overviewpage.h | 1 + src/qt/transactionfilterproxy.cpp | 5 ++++ src/qt/transactionfilterproxy.h | 3 +++ src/qt/transactiontablemodel.cpp | 3 ++- 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index e6d523d960..c4efa2c0cf 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -161,16 +161,45 @@ void OverviewPage::showEvent(QShowEvent *event) updateTransactions(); } +int OverviewPage::getNumTransactionsForView() +{ + // Compute the maximum number of transactions the transaction list widget + // can hold without overflowing. + const size_t itemHeight = DECORATION_SIZE + ui->listTransactions->spacing(); + const size_t contentsHeight = ui->listTransactions->height(); + const int numItems = contentsHeight / itemHeight; + + return numItems; +} + + void OverviewPage::updateTransactions() { - if(filter) + if (filter) { - // Show the maximum number of transactions the transaction list widget - // can hold without overflowing. - const size_t itemHeight = DECORATION_SIZE + ui->listTransactions->spacing(); - const size_t contentsHeight = ui->listTransactions->height(); - const size_t numItems = contentsHeight / itemHeight; - filter->setLimit(numItems); + int numItems = getNumTransactionsForView(); + + // This is a "stairstep" approach, using x3 to x6 factors to size the getLimit. + // Based on testing with a wallet with a large number of transactions (40k+) + // Using a factor of three is a good balance between the setRowHidden loop + // and the very high expense of the getLimit call, which invalidates the filter + // and sort, and implicitly redoes the sort, which can take seconds. + + // Most main window resizes will be done without an actual call to getLimit. + if (filter->getLimit() < numItems) + { + filter->setLimit(numItems * 3); + } + else if (filter->getLimit() > numItems * 6) + { + filter->setLimit(numItems * 3); + } + + for (int i = 0; i <= filter->getLimit(); ++i) + { + ui->listTransactions->setRowHidden(i, i >= numItems); + } + ui->listTransactions->update(); } } @@ -240,8 +269,8 @@ void OverviewPage::setWalletModel(WalletModel *model) filter->setDynamicSortFilter(true); filter->setSortRole(Qt::EditRole); filter->setShowInactive(false); + filter->setLimit(getNumTransactionsForView()); filter->sort(TransactionTableModel::Status, Qt::DescendingOrder); - ui->listTransactions->setModel(filter.get()); ui->listTransactions->setModelColumn(TransactionTableModel::ToAddress); diff --git a/src/qt/overviewpage.h b/src/qt/overviewpage.h index f3e131f9a8..41f36a8935 100644 --- a/src/qt/overviewpage.h +++ b/src/qt/overviewpage.h @@ -47,6 +47,7 @@ public slots: void showEvent(QShowEvent *event); private: + int getNumTransactionsForView(); void updateTransactions(); Ui::OverviewPage *ui; diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index 3cea26579a..7cb7498fff 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -87,6 +87,11 @@ void TransactionFilterProxy::setLimit(int limit) invalidate(); } +int TransactionFilterProxy::getLimit() +{ + return this->limitRows; +} + void TransactionFilterProxy::setShowInactive(bool showInactive) { this->showInactive = showInactive; diff --git a/src/qt/transactionfilterproxy.h b/src/qt/transactionfilterproxy.h index 2c86f62ab8..429ce1407c 100644 --- a/src/qt/transactionfilterproxy.h +++ b/src/qt/transactionfilterproxy.h @@ -31,6 +31,9 @@ class TransactionFilterProxy : public QSortFilterProxyModel /** Set maximum number of rows returned, -1 if unlimited. */ void setLimit(int limit); + /** Get maximum number of rows returned, -1 if unlimited. */ + int getLimit(); + /** Set whether to show conflicted transactions. */ void setShowInactive(bool showInactive); diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 5e9b9fde0f..6f2e292e62 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -115,7 +115,8 @@ class TransactionTablePriv status = CT_DELETED; /* In model, but want to hide, treat as deleted */ } - LogPrint(BCLog::LogFlags::VERBOSE, " inWallet=%i inModel=%i Index=%i-%i showTransaction=%i derivedStatus=%i", inWallet, inModel, lowerIndex, upperIndex, showTransaction, status); + LogPrint(BCLog::LogFlags::VERBOSE, " inWallet=%i inModel=%i Index=%i-%i showTransaction=%i derivedStatus=%i", + inWallet, inModel, lowerIndex, upperIndex, showTransaction, status); switch(status) From e27b2bd97e661e6946288e22bba6e0f5c4ce57ad Mon Sep 17 00:00:00 2001 From: jamescowens Date: Mon, 20 Jul 2020 21:07:59 -0400 Subject: [PATCH 2/3] Add transactionUpdated signal With the hide loop in the updatedTransactions method in the overview page, the updateTransactions needs to be called after the underlying transaction model has recevied a new row, because the getLimit is set to a higher value than is visible on purpose to avoid a resort. The signal that would normally be used, numTransactionsChanged, does NOT work for this purpose, because that signal is only sent when the size of the number of transactions in the wallet is changed. The size will change if the wallet is just keys and a resync or rescan is done, but the situation when an old wallet is used with existing transactions, but the blockchain is synced from zero will not work. The wallet size is constant then for old transactions relinked. To handle this case I had to add a new signal transactionUpdated which is sent when WalletModel::updateTransaction is called. --- src/qt/overviewpage.cpp | 3 +++ src/qt/overviewpage.h | 2 +- src/qt/walletmodel.cpp | 12 ++++++++++-- src/qt/walletmodel.h | 5 +++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index c4efa2c0cf..204c2c91be 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -279,6 +279,9 @@ void OverviewPage::setWalletModel(WalletModel *model) connect(model, SIGNAL(balanceChanged(qint64, qint64, qint64, qint64)), this, SLOT(setBalance(qint64, qint64, qint64, qint64))); connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); + + connect(model, SIGNAL(transactionUpdated()), this, SLOT(updateTransactions())); + UpdateBoincUtilization(); } diff --git a/src/qt/overviewpage.h b/src/qt/overviewpage.h index 41f36a8935..6c870e585f 100644 --- a/src/qt/overviewpage.h +++ b/src/qt/overviewpage.h @@ -48,7 +48,6 @@ public slots: private: int getNumTransactionsForView(); - void updateTransactions(); Ui::OverviewPage *ui; ResearcherModel *researcherModel; @@ -63,6 +62,7 @@ public slots: private slots: void updateDisplayUnit(); + void updateTransactions(); void updateResearcherStatus(); void updateMagnitude(); void updatePendingAccrual(); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 41be1f564f..cce29d8791 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -126,16 +126,24 @@ void WalletModel::checkBalanceChanged() void WalletModel::updateTransaction(const QString &hash, int status) { - if(transactionTableModel) + if (transactionTableModel) + { transactionTableModel->updateTransaction(hash, status); + // Note this is subtly different than the below. If a resync is being done on a wallet + // that already has transactions, the numTransactionsChanged will not be emitted after the + // wallet is loaded because the size() does not change. See the comments in the header file. + emit transactionUpdated(); + } + // Balance and number of transactions might have changed checkBalanceChanged(); int newNumTransactions = getNumTransactions(); - if(cachedNumTransactions != newNumTransactions) + if (cachedNumTransactions != newNumTransactions) { cachedNumTransactions = newNumTransactions; + emit numTransactionsChanged(newNumTransactions); } } diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index ec65ce8279..4f6580ac8e 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -163,6 +163,11 @@ public slots: void pollBalanceChanged(); signals: + // Transaction updated. This is necessary because on a resync from zero with an existing wallet. + // the numTransactionsChanged signal will not be emitted, and therefore the overpage transaction list + // needs this signal instead. + void transactionUpdated(); + // Signal that balance in wallet changed void balanceChanged(qint64 balance, qint64 stake, qint64 unconfirmedBalance, qint64 immatureBalance); From ff026e51a8ec5c91134e8cb6fbcf6753344a0791 Mon Sep 17 00:00:00 2001 From: jamescowens Date: Wed, 22 Jul 2020 17:06:25 -0400 Subject: [PATCH 3/3] Correct comments --- src/qt/overviewpage.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 204c2c91be..ea5c4d40e0 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -179,13 +179,14 @@ void OverviewPage::updateTransactions() { int numItems = getNumTransactionsForView(); - // This is a "stairstep" approach, using x3 to x6 factors to size the getLimit. + // This is a "stairstep" approach, using x3 to x6 factors to size the setLimit. // Based on testing with a wallet with a large number of transactions (40k+) // Using a factor of three is a good balance between the setRowHidden loop // and the very high expense of the getLimit call, which invalidates the filter - // and sort, and implicitly redoes the sort, which can take seconds. + // and sort, and implicitly redoes the sort, which can take seconds for a large + // wallet. - // Most main window resizes will be done without an actual call to getLimit. + // Most main window resizes will be done without an actual call to setLimit. if (filter->getLimit() < numItems) { filter->setLimit(numItems * 3);