relax GUI locks: avoid unnecesarry calls to ::ChainstateActive().IsInitialBlockDownload #19007
Conversation
…itialBlockDownload() Introduced in bitcoin#12783 / 1e0f3c4
jonasschnelli
force-pushed
the
2020/05/fix_macguilock
branch
from
feecf49 to
ec2c94f
Compare
There was a problem hiding this comment.
Code review ACK feecf490c418399e9771a5b48e078a4c2c5b6b55. It's good to avoid calls from gui->node in the middle of node->gui callbacks
EDIT: Updated ACK comment in light of fix from other PR #19007 (comment)
| void setNetworkActive(bool networkActive); | ||
| /** Set number of blocks and last block date shown in the UI */ | ||
| void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool headers); | ||
| void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool headers, bool initial_sync); |
There was a problem hiding this comment.
In commit "relax GUI locks: avoid unnecesarry calls to ::ChainstateActive().IsInitialBlockDownload()" (feecf490c418399e9771a5b48e078a4c2c5b6b55)
Could revert all these changes to rpcconsole.h and rpcconsole.cpp. Qt is fine with dropping ununused signal arguments https://doc.qt.io/qt-5/signalsandslots.html "a slot may have a shorter signature than the signal it receives"
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Thanks @ryanofsky for the review and pointing to #18152. I agree that #18152 (which I wasn't aware) is the superior solution. |
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
I think this can be closed since OG alternative #18152 was merged |
386ec19 Reduce cs_main lock accumulation during GUI startup (Jonas Schnelli) d42cb79 Optionally populate BlockAndHeaderTipInfo during AppInitMain (Jonas Schnelli) b354a14 Add BlockAndHeaderTipInfo to the node interface/appInit (Jonas Schnelli) 25e1d0b RPCConsole, take initial chaintip data as parameter (Jonas Schnelli) Pull request description: During the GUI startup, there is currently an accumulation of cs_main locks due to setting initial chain state values at multiple locations (in the GUI main thread). This PR tries to cache the initial chain state (tip height, tip time, best header, etc.) short after loading the blockindex. The cached values are then used instead of fetching them again (and thus locking `cs_main`) during setting the client model. This should fix the initial GUI blocking often experienced during or short after the splashscreen. On mac, best tested together with #19007. ACKs for top commit: promag: Code review ACK 386ec19. ryanofsky: Code review ACK 386ec19. Just rebased since last review due to conflicts Tree-SHA512: caccca05360e6dc0c3aade5e7ed24be513607821a8bd6612d0337259304ab772799fb2d707a0d7c7e50fbff4bd394354643fd0aeaa3bb55960ccc28562f4763d
5 participants
Introduced in #12783 / 1e0f3c4
Calling
m_node.isInitialBlockDownload()during catch-up in the main GUI thread leads to locking the GUI for seconds,... which is avoidable since we fetch that information via a signal anyways.