From abac8c2b783d69d3483034710d62ae965eee9ac6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 6 Feb 2021 15:36:40 +0300 Subject: [PATCH 1/5] instantsend|sigs: Sleep when there is no more work Instead of sleeping only when no work has been done. Avoids useless cycles, improves batching. --- src/llmq/quorums_instantsend.cpp | 24 ++++++++++-------------- src/llmq/quorums_instantsend.h | 2 +- src/llmq/quorums_signing.cpp | 2 +- src/llmq/quorums_signing_shares.cpp | 20 ++++++++------------ src/llmq/quorums_signing_shares.h | 2 +- 5 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 15cf51b80c1d..1bbbb1d376a1 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -731,6 +731,7 @@ bool CInstantSendManager::PreVerifyInstantSendLock(const llmq::CInstantSendLock& bool CInstantSendManager::ProcessPendingInstantSendLocks() { decltype(pendingInstantSendLocks) pend; + bool more_work{false}; { LOCK(cs); @@ -745,6 +746,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() pend.emplace(it->first, std::move(it->second)); pendingInstantSendLocks.erase(it); } + more_work = true; } } @@ -776,7 +778,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() ProcessPendingInstantSendLocks(dkgInterval, pend, true); } - return true; + return more_work; } std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map, StaticSaltedHasher>& pend, bool ban) @@ -1365,7 +1367,7 @@ void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) } } -bool CInstantSendManager::ProcessPendingRetryLockTxs() +void CInstantSendManager::ProcessPendingRetryLockTxs() { decltype(pendingRetryTxs) retryTxs; { @@ -1374,11 +1376,11 @@ bool CInstantSendManager::ProcessPendingRetryLockTxs() } if (retryTxs.empty()) { - return false; + return; } if (!IsInstantSendEnabled()) { - return false; + return; } int retryCount = 0; @@ -1428,8 +1430,6 @@ bool CInstantSendManager::ProcessPendingRetryLockTxs() LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- retried %d TXs. nonLockedTxs.size=%d\n", __func__, retryCount, nonLockedTxs.size()); } - - return retryCount != 0; } bool CInstantSendManager::AlreadyHave(const CInv& inv) @@ -1521,15 +1521,11 @@ size_t CInstantSendManager::GetInstantSendLockCount() void CInstantSendManager::WorkThreadMain() { while (!workInterrupt) { - bool didWork = false; - - didWork |= ProcessPendingInstantSendLocks(); - didWork |= ProcessPendingRetryLockTxs(); + bool more_work = ProcessPendingInstantSendLocks(); + ProcessPendingRetryLockTxs(); - if (!didWork) { - if (!workInterrupt.sleep_for(std::chrono::milliseconds(100))) { - return; - } + if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + return; } } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 8e5bbcc7caf9..2b372d91b4ee 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -159,7 +159,7 @@ class CInstantSendManager : public CRecoveredSigsListener void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock); void RemoveChainLockConflictingLock(const uint256& islockHash, const CInstantSendLock& islock); static void AskNodesForLockedTx(const uint256& txid); - bool ProcessPendingRetryLockTxs(); + void ProcessPendingRetryLockTxs(); bool AlreadyHave(const CInv& inv); bool GetInstantSendLockByHash(const uint256& hash, CInstantSendLock& ret); diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 70cbc5d29fc1..c09f31ba4834 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -675,7 +675,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs() } } - return true; + return verifyCount >= 32; } // signature must be verified already diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 8859441d481e..52817e4d7bce 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -704,7 +704,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) ProcessPendingSigShares(v, quorums, connman); } - return true; + return verifyCount >= 32; } // It's ensured that no duplicates are passed to this method @@ -1501,12 +1501,12 @@ void CSigSharesManager::WorkThreadMain() continue; } - bool didWork = false; + bool more_work = false; RemoveBannedNodeStates(); - didWork |= quorumSigningManager->ProcessPendingRecoveredSigs(); - didWork |= ProcessPendingSigShares(*g_connman); - didWork |= SignPendingSigShares(); + more_work |= quorumSigningManager->ProcessPendingRecoveredSigs(); + more_work |= ProcessPendingSigShares(*g_connman); + SignPendingSigShares(); if (GetTimeMillis() - lastSendTime > 100) { SendMessages(); @@ -1517,10 +1517,8 @@ void CSigSharesManager::WorkThreadMain() quorumSigningManager->Cleanup(); // TODO Wakeup when pending signing is needed? - if (!didWork) { - if (!workInterrupt.sleep_for(std::chrono::milliseconds(100))) { - return; - } + if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + return; } } } @@ -1531,7 +1529,7 @@ void CSigSharesManager::AsyncSign(const CQuorumCPtr& quorum, const uint256& id, pendingSigns.emplace_back(quorum, id, msgHash); } -bool CSigSharesManager::SignPendingSigShares() +void CSigSharesManager::SignPendingSigShares() { std::vector> v; { @@ -1557,8 +1555,6 @@ bool CSigSharesManager::SignPendingSigShares() } } } - - return !v.empty(); } CSigShare CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index 695fcac5b88c..898a0b5e69c9 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/src/llmq/quorums_signing_shares.h @@ -451,7 +451,7 @@ class CSigSharesManager : public CRecoveredSigsListener void CollectSigSharesToSend(std::unordered_map>& sigSharesToSend); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes); void CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce); - bool SignPendingSigShares(); + void SignPendingSigShares(); void WorkThreadMain(); }; From b6b5ccc994a86e02397dcd9bb2101bfba99ee706 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 10 Feb 2021 04:27:58 +0100 Subject: [PATCH 2/5] llmq: Add and use nMaxBatchSize --- src/llmq/quorums_signing.cpp | 5 +++-- src/llmq/quorums_signing_shares.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index c09f31ba4834..b5c5a225e6af 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -621,7 +621,8 @@ bool CSigningManager::ProcessPendingRecoveredSigs() ProcessPendingReconstructedRecoveredSigs(); - CollectPendingRecoveredSigsToVerify(32, recSigsByNode, quorums); + const size_t nMaxBatchSize{32}; + CollectPendingRecoveredSigsToVerify(nMaxBatchSize, recSigsByNode, quorums); if (recSigsByNode.empty()) { return false; } @@ -675,7 +676,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs() } } - return verifyCount >= 32; + return verifyCount >= nMaxBatchSize; } // signature must be verified already diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 52817e4d7bce..fd43beef1662 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -639,7 +639,8 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) std::unordered_map> sigSharesByNodes; std::unordered_map, CQuorumCPtr, StaticSaltedHasher> quorums; - CollectPendingSigSharesToVerify(32, sigSharesByNodes, quorums); + const size_t nMaxBatchSize{32}; + CollectPendingSigSharesToVerify(nMaxBatchSize, sigSharesByNodes, quorums); if (sigSharesByNodes.empty()) { return false; } @@ -704,7 +705,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) ProcessPendingSigShares(v, quorums, connman); } - return verifyCount >= 32; + return verifyCount >= nMaxBatchSize; } // It's ensured that no duplicates are passed to this method From eb2fc1279d68aa52eb3dbc62769f2bd251ae08a7 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 10 Feb 2021 04:30:54 +0100 Subject: [PATCH 3/5] llmq: Compare to what we got in return, not what we verified at the end It might happen that we get 32 pending but do only verify less than 32 and in this case we would assume there is no more work but it could still be more in the pipeline from my understanding. --- src/llmq/quorums_signing.cpp | 2 +- src/llmq/quorums_signing_shares.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index b5c5a225e6af..34c7a6f54ad8 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -676,7 +676,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs() } } - return verifyCount >= nMaxBatchSize; + return recSigsByNode.size() >= nMaxBatchSize; } // signature must be verified already diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index fd43beef1662..083f87de469a 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -705,7 +705,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) ProcessPendingSigShares(v, quorums, connman); } - return verifyCount >= nMaxBatchSize; + return sigSharesByNodes.size() >= nMaxBatchSize; } // It's ensured that no duplicates are passed to this method From 9c7ffd3fae35b3b9a54418caea4d6df7a9bcff2c Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 10 Feb 2021 04:32:03 +0100 Subject: [PATCH 4/5] llmq: Rename more_work -> fMoreWork --- src/llmq/quorums_instantsend.cpp | 10 +++++----- src/llmq/quorums_signing_shares.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 1bbbb1d376a1..4b40998f777f 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -731,7 +731,7 @@ bool CInstantSendManager::PreVerifyInstantSendLock(const llmq::CInstantSendLock& bool CInstantSendManager::ProcessPendingInstantSendLocks() { decltype(pendingInstantSendLocks) pend; - bool more_work{false}; + bool fMoreWork{false}; { LOCK(cs); @@ -746,7 +746,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() pend.emplace(it->first, std::move(it->second)); pendingInstantSendLocks.erase(it); } - more_work = true; + fMoreWork = true; } } @@ -778,7 +778,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() ProcessPendingInstantSendLocks(dkgInterval, pend, true); } - return more_work; + return fMoreWork; } std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map, StaticSaltedHasher>& pend, bool ban) @@ -1521,10 +1521,10 @@ size_t CInstantSendManager::GetInstantSendLockCount() void CInstantSendManager::WorkThreadMain() { while (!workInterrupt) { - bool more_work = ProcessPendingInstantSendLocks(); + bool fMoreWork = ProcessPendingInstantSendLocks(); ProcessPendingRetryLockTxs(); - if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { return; } } diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 083f87de469a..bd01dbe9a614 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -1502,11 +1502,11 @@ void CSigSharesManager::WorkThreadMain() continue; } - bool more_work = false; + bool fMoreWork = false; RemoveBannedNodeStates(); - more_work |= quorumSigningManager->ProcessPendingRecoveredSigs(); - more_work |= ProcessPendingSigShares(*g_connman); + fMoreWork |= quorumSigningManager->ProcessPendingRecoveredSigs(); + fMoreWork |= ProcessPendingSigShares(*g_connman); SignPendingSigShares(); if (GetTimeMillis() - lastSendTime > 100) { @@ -1518,7 +1518,7 @@ void CSigSharesManager::WorkThreadMain() quorumSigningManager->Cleanup(); // TODO Wakeup when pending signing is needed? - if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { return; } } From 97bc79bb21a8967238d64f476ab480c0972f3246 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Wed, 10 Feb 2021 04:34:11 +0100 Subject: [PATCH 5/5] llmq: Be consistent with the other fMoreWork initialization --- src/llmq/quorums_signing_shares.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index bd01dbe9a614..35231df25603 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -1502,7 +1502,7 @@ void CSigSharesManager::WorkThreadMain() continue; } - bool fMoreWork = false; + bool fMoreWork{false}; RemoveBannedNodeStates(); fMoreWork |= quorumSigningManager->ProcessPendingRecoveredSigs();