Skip to content

[Validation] Reduce cs_main locks during ConnectTip/SyncWithWallets #1721

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
Merged
random-zebra merged 7 commits into from
Aug 5, 2020
Merged

[Validation] Reduce cs_main locks during ConnectTip/SyncWithWallets #1721

random-zebra merged 7 commits into from
Aug 5, 2020

Conversation

@furszy
Copy link

@furszy furszy commented Jul 1, 2020

Work done on top of #1717 .

Essentially an improvement over SyncWithWallets & SyncTransaction flow to run without locking cs_main.
(made as less modifications as possible over the zpiv wallet/chain files, no real need to improve that code.. most of it will disappear soon)

It's an adapted version of bitcoin#7946.

@furszy furszy self-assigned this Jul 1, 2020
@furszy furszy force-pushed the 2020_05_wallet_reduce_locks branch from 089c5a9 to 2be4426 Compare July 1, 2020 07:55
@furszy furszy changed the title [WIP] Reduce cs_main locks during ConnectTip/SyncWithWallets Reduce cs_main locks during ConnectTip/SyncWithWallets Jul 1, 2020
@furszy furszy force-pushed the 2020_05_wallet_reduce_locks branch from 46dd507 to 610c7f7 Compare July 12, 2020 00:28
@furszy furszy changed the title Reduce cs_main locks during ConnectTip/SyncWithWallets [Validation] Reduce cs_main locks during ConnectTip/SyncWithWallets Jul 26, 2020
@random-zebra random-zebra added this to the 5.0.0 milestone Jul 29, 2020
@furszy furszy requested a review from random-zebra July 31, 2020 01:30
random-zebra
random-zebra suggested changes Aug 1, 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.

Seems we are introducing back the unused return value of SetMerkleBranch (and relative chainActive part, bringing back the need for AssertLockHeld(cs_main)).

Someone had removed those already in #1520 :)

furszy and others added 5 commits August 1, 2020 12:52
@furszy
Remove CWalletDB* parameter from CWallet::AddToWallet
d8cc80a 
Coming from btc@867f842f1e5a385aeb2093f802d6f37a84d0fe5d
@furszy
Remove unused pwalletdb from CommitTransaction
5e155b4 
Coming from btc@5723bb44ce2c6bb14114aa7f211160702a47ac91
@furszy
Reduce cs_main locks during ConnectTip/SyncWithWallets
f1eb073 
Adapted work coming from btc@b3b3c2a5623d5c942d2b3565cc2d833c65105555
@domob1812 @furszy
trivial: remove unused variable
1fe0be6 
Remove the unused variable "blockTmp" in CMerkleTx::SetMerkleBranch.  It
was previously used to read the block from disk if not provided as
argument, but is no longer needed.
@furszy
Performance Regression Fix: Pre-Allocate txChanged vector
3b2807f 
coming from btc@ec81881b86b9680fcdcc42fd3ba31f04b8d09714
@furszy furszy force-pushed the 2020_05_wallet_reduce_locks branch from 610c7f7 to 3b2807f Compare August 1, 2020 17:34
@furszy
Copy link
Author

furszy commented Aug 1, 2020

Update per feedback. One more section without cs_main locked :) .

@furszy
Copy link
Author

furszy commented Aug 2, 2020

This will need more work.
Further on upstream, bitcoin#9583 reverts part of the changes made in f1eb073.
But.. to get to that point, need to continue moving forward with plenty other back ports.

@random-zebra
Copy link

random-zebra commented Aug 2, 2020

Looking good.
Just for tracking, only f1eb073 is backported from bitcoin#7946. As for the others:

@random-zebra random-zebra mentioned this pull request Aug 2, 2020
Merged
random-zebra
random-zebra previously approved these changes Aug 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.

tested ACK 4b4858b
(didn't test the zerocoin changes though)

@furszy
Move wallet callbacks into cs_main.
9d1beb3 
adaptation of btc@989989354b68d3954fe2742b96c53eeb2e8a7670
random-zebra
random-zebra approved these changes Aug 4, 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.

utACK 9d1beb3

Fuzzbawls
Fuzzbawls approved these changes Aug 5, 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 9d1beb3

@random-zebra random-zebra merged commit 8e19562 into PIVX-Project:master Aug 5, 2020
@random-zebra random-zebra modified the milestones: 5.0.0, 4.3.0 Sep 10, 2020
@furszy furszy deleted the 2020_05_wallet_reduce_locks branch November 29, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

+1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

Assignees

@furszy furszy

Labels

Refactoring Upstream

Projects

None yet

Milestone

4.3.0

Development

Successfully merging this pull request may close these issues.

5 participants

@furszy @random-zebra @Fuzzbawls @domob1812 @TheBlueMatt