Merge #18160: gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged

0933a37078 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa)

Pull request description:

  Each 250ms the slot `WalletModel::pollBalanceChanged` is called which, at worst case, calls `Wallet::GetBalance`. This is a waste of resources since most of the time there aren't new transactions or new blocks. Fix this by early checking if cache is dirty or not.

  The actual balance computation can still hang the GUI thread but that is tracked in #16874 and should be fixed with a solution similar to #17135.

ACKs for top commit:
  hebasto:
    ACK 0933a37078, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
  jonasschnelli:
    utACK 0933a37078
  instagibbs:
    ACK 0933a37078
  ryanofsky:
    Code review ACK 0933a37078, but I would prefer (not strongly) for #17905 to be merged first. This PR can be simpler if it is based on #17905, so tryGetBalances can just be left alone instead of changing into to a more complicated tryGetBalancesIfNeeded function, and then getting changed back later when we want to optimize it out.
  jonatack:
    ACK 0933a37078 based primarily on code review, despite a lot of manual testing with a large 177MB wallet.

Tree-SHA512: 18db35bf33a7577666658c8cb0b57308c8474baa5ea95bf1468cd8531a69857d8915584f6ac505874717aa6aabeb1b506ac77630f8acdb6651afab89275e38a1
This commit is contained in:
Wladimir J. van der Laan 2020-03-31 13:48:52 +02:00
commit f2880e21ef
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
3 changed files with 15 additions and 14 deletions

View file

@ -368,16 +368,17 @@ public:
}
return result;
}
bool tryGetBalances(WalletBalances& balances, int& num_blocks) override
bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override
{
auto locked_chain = m_wallet->chain().lock(true /* try_lock */);
if (!locked_chain) return false;
num_blocks = locked_chain->getHeight().get_value_or(-1);
if (!force && num_blocks == cached_num_blocks) return false;
TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) {
return false;
}
balances = getBalances();
num_blocks = locked_chain->getHeight().get_value_or(-1);
return true;
}
CAmount getBalance() override { return m_wallet->GetBalance().m_mine_trusted; }

View file

@ -201,8 +201,11 @@ public:
//! Get balances.
virtual WalletBalances getBalances() = 0;
//! Get balances if possible without blocking.
virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0;
//! Get balances if possible without waiting for chain and wallet locks.
virtual bool tryGetBalances(WalletBalances& balances,
int& num_blocks,
bool force,
int cached_num_blocks) = 0;
//! Get balance.
virtual CAmount getBalance() = 0;

View file

@ -78,21 +78,18 @@ void WalletModel::pollBalanceChanged()
// rescan.
interfaces::WalletBalances new_balances;
int numBlocks = -1;
if (!m_wallet->tryGetBalances(new_balances, numBlocks)) {
if (!m_wallet->tryGetBalances(new_balances, numBlocks, fForceCheckBalanceChanged, cachedNumBlocks)) {
return;
}
if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks)
{
fForceCheckBalanceChanged = false;
fForceCheckBalanceChanged = false;
// Balance and number of transactions might have changed
cachedNumBlocks = numBlocks;
// Balance and number of transactions might have changed
cachedNumBlocks = numBlocks;
checkBalanceChanged(new_balances);
if(transactionTableModel)
transactionTableModel->updateConfirmations();
}
checkBalanceChanged(new_balances);
if(transactionTableModel)
transactionTableModel->updateConfirmations();
}
void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances)