refactor: create helper function RelayRecoveredSig inside peerman #6423
Conversation
PastaPastaPasta
force-pushed
the
refactor-net-min-m_nodes_mutex-contention-llmq
branch
from
9ef40dc to
09a483d
Compare
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6423.09a483dc. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6423.09a483dc. The image should be on dockerhub soon. |
|
Okay, tested on a testnet node; and appears to work as expected. Now, disclaimer, this is on 3 testnet nodes. One running rc.1, one running this PR, and one running 6422. It is a bit hard to compare between rc.1 and the PRs as the PRs are built using debug, so their "contended time" is going to be higher, even if actual contention should be lower. The total percentage of contentions caused by m_nodes_mutex went from ~41% -> 26%. It did result in a significantly increased contention over But the "contended time" for m_nodes_mutex went from 896839 -> 316444 = 580,395 |
this commit should have a few benefits: 1. previous logic using ForEachNode results in locking m_nodes_mutex, a highly contended RecursiveMutex, AND m_peer_mutex(in GetPeerRef) 2. prior also resulted in calling .find over the m_peer_map for each node. Basically old logic was (probably) O(n(nlogn) the new logic results in acquiring m_peer_mutex once and looping over the list of peers, (probably) O(n) 3. Moves networking logic out of llmq/ and into actual net_processing.cpp
PastaPastaPasta
force-pushed
the
refactor-net-min-m_nodes_mutex-contention-llmq
branch
from
09a483d to
4668db6
Compare
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6423.4668db60. A new comment will be made when the image is pushed. |
| @@ -633,12 +633,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered | |||
| WITH_LOCK(cs_pending, pendingReconstructedRecoveredSigs.erase(recoveredSig->GetHash())); | |||
|
|
|||
| if (m_mn_activeman != nullptr) { | |||
| CInv inv(MSG_QUORUM_RECOVERED_SIG, recoveredSig->GetHash()); | |||
| connman.ForEachNode([&](const CNode* pnode) { | |||
There was a problem hiding this comment.
oh this change is soo good! Look to this one: 6149288
Maybe you can drop all usages of CConnman from llmq code?
There was a problem hiding this comment.
Aye! Awesome, didn't even realize I'd removed all usages
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6423.4668db60. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v22.1.0-devpr6423.86e92c37. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.1.0-devpr6423.86e92c37. The image should be on dockerhub soon. |
Issue being fixed or feature implemented
High m_nodes_mutex lock contention during high load
What was done?
this commit should have a few benefits:
How Has This Been Tested?
Hasn't really yet; it builds, but I need to run tests / maybe deploy to testnet mn
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.