Skip to content

Commit 2d84c2b

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6906: refactor: optimize AsyncSign by using std::move for quorum parameter
6b49730 refactor: use non-const reference for pQuorum in quorum iteration loop to allow moves (pasta) 35a4f07 refactor: replace CQuorumCPtr with CQuorum references in quorum-related functions (pasta) 382fd3c refactor: use std::move for CQuorumCPtr in StartCachePopulatorThread (pasta) 5a6e64d refactor: use std::move for CQuorumCPtr in StartQuorumDataRecoveryThread (pasta) 46856fc refactor: update RequestQuorumData and GetQuorumRecoveryStartOffset to use CQuorum reference (pasta) 5e888c4 refactor: optimize AsyncSign by using std::move for quorum parameter (pasta) Pull request description: ## Issue being fixed or feature implemented Using std::move here should save on the magnitude of ~10-150 ns depending on how contended the underlying shared_ptr is. This move becomes basically free, with no atomics etc being touched. Currently, we pass a const ref in, then copy it, resulting in 2 unneeded atomic operations (++ for the copy, -- for the destroy of the one we could've just moved in. ## What was done? use std::move Also; prefer just using the reference to CQuorum where possible, as opposed to the shared_ptr ## How Has This Been Tested? compiles ## Breaking Changes _Please describe any breaking changes your code introduces_ ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 6b49730 Tree-SHA512: f187d81d191fec935f7ca533513bdf56be810c8eb548509c3de1dbcf09431c49178352c8d94523378d0edfeedbccda0543328a331df514efcb303bf658a55ec2
2 parents d56bff5 + 6b49730 commit 2d84c2b

5 files changed

Lines changed: 82 additions & 82 deletions

File tree

‎src/llmq/quorums.cpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C
260260
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Process block %s\n", __func__, pIndex->GetBlockHash().ToString());
261261

262262
for (const auto& params : Params().GetConsensus().llmqs) {
263-
const auto vecQuorums = ScanQuorums(params.type, pIndex, params.keepOldConnections);
263+
auto vecQuorums = ScanQuorums(params.type, pIndex, params.keepOldConnections);
264264

265265
// First check if we are member of any quorum of this type
266266
const uint256 proTxHash = m_mn_activeman != nullptr ? m_mn_activeman->GetProTxHash() : uint256();
@@ -269,7 +269,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C
269269
return pQuorum->IsValidMember(proTxHash);
270270
});
271271

272-
for (const auto& pQuorum : vecQuorums) {
272+
for (auto& pQuorum : vecQuorums) {
273273
// If there is already a thread running for this specific quorum skip it
274274
if (pQuorum->fQuorumDataRecoveryThreadRunning) {
275275
continue;
@@ -296,7 +296,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C
296296
}
297297

298298
// Finally start the thread which triggers the requests for this quorum
299-
StartQuorumDataRecoveryThread(connman, pQuorum, pIndex, nDataMask);
299+
StartQuorumDataRecoveryThread(connman, std::move(pQuorum), pIndex, nDataMask);
300300
}
301301
}
302302
}
@@ -478,7 +478,7 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const CQuorumBlockP
478478
return quorum_block_processor.HasMinedCommitment(llmqType, quorumHash);
479479
}
480480

481-
bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, CQuorumCPtr pQuorum, uint16_t nDataMask,
481+
bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorum& quorum, uint16_t nDataMask,
482482
const uint256& proTxHash) const
483483
{
484484
if (pfrom == nullptr) {
@@ -489,12 +489,12 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, CQuorumC
489489
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pfrom is not a verified masternode\n", __func__);
490490
return false;
491491
}
492-
const Consensus::LLMQType llmqType = pQuorum->qc->llmqType;
492+
const Consensus::LLMQType llmqType = quorum.qc->llmqType;
493493
if (!Params().GetLLMQ(llmqType).has_value()) {
494494
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid llmqType: %d\n", __func__, ToUnderlying(llmqType));
495495
return false;
496496
}
497-
const CBlockIndex* pindex{pQuorum->m_quorum_base_block_index};
497+
const CBlockIndex* pindex{quorum.m_quorum_base_block_index};
498498
if (pindex == nullptr) {
499499
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid m_quorum_base_block_index : nullptr\n", __func__);
500500
return false;
@@ -676,7 +676,7 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, gsl::not_nul
676676
return BuildQuorumFromCommitment(llmqType, pQuorumBaseBlockIndex, populate_cache);
677677
}
678678

679-
size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, const CBlockIndex* pIndex) const
679+
size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const
680680
{
681681
assert(m_mn_activeman);
682682

@@ -698,7 +698,7 @@ size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, c
698698
}
699699
}
700700
}
701-
return nIndex % pQuorum->qc->validMembers.size();
701+
return nIndex % quorum.qc->validMembers.size();
702702
}
703703

704704
MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& connman, std::string_view msg_type, CDataStream& vRecv)
@@ -886,7 +886,7 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c
886886
return {};
887887
}
888888

889-
void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const
889+
void CQuorumManager::StartCachePopulatorThread(CQuorumCPtr pQuorum) const
890890
{
891891
if (!pQuorum->HasVerificationVector()) {
892892
return;
@@ -899,7 +899,7 @@ void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const
899899
pQuorum->m_quorum_base_block_index->GetBlockHash().ToString());
900900

901901
// when then later some other thread tries to get keys, it will be much faster
902-
workerPool.push([pQuorum, t, this](int threadId) {
902+
workerPool.push([pQuorum = std::move(pQuorum), t, this](int threadId) {
903903
for (const auto i : irange::range(pQuorum->members.size())) {
904904
if (quorumThreadInterrupt) {
905905
break;
@@ -916,7 +916,7 @@ void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const
916916
});
917917
}
918918

919-
void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuorumCPtr pQuorum,
919+
void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, CQuorumCPtr pQuorum,
920920
const CBlockIndex* pIndex, uint16_t nDataMaskIn) const
921921
{
922922
assert(m_mn_activeman);
@@ -927,13 +927,13 @@ void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuo
927927
return;
928928
}
929929

930-
workerPool.push([&connman, pQuorum, pIndex, nDataMaskIn, this](int threadId) {
930+
workerPool.push([&connman, pQuorum = std::move(pQuorum), pIndex, nDataMaskIn, this](int threadId) {
931931
size_t nTries{0};
932932
uint16_t nDataMask{nDataMaskIn};
933933
int64_t nTimeLastSuccess{0};
934934
uint256* pCurrentMemberHash{nullptr};
935935
std::vector<uint256> vecMemberHashes;
936-
const size_t nMyStartOffset{GetQuorumRecoveryStartOffset(pQuorum, pIndex)};
936+
const size_t nMyStartOffset{GetQuorumRecoveryStartOffset(*pQuorum, pIndex)};
937937
const int64_t nRequestTimeout{10};
938938

939939
auto printLog = [&](const std::string& strMessage) {
@@ -1009,7 +1009,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuo
10091009
return;
10101010
}
10111011

1012-
if (RequestQuorumData(pNode, connman, pQuorum, nDataMask, proTxHash)) {
1012+
if (RequestQuorumData(pNode, connman, *pQuorum, nDataMask, proTxHash)) {
10131013
nTimeLastSuccess = GetTime<std::chrono::seconds>().count();
10141014
printLog("Requested");
10151015
} else {

‎src/llmq/quorums.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ class CQuorumManager
287287

288288
static bool HasQuorum(Consensus::LLMQType llmqType, const CQuorumBlockProcessor& quorum_block_processor, const uint256& quorumHash);
289289

290-
bool RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorumCPtr pQuorum, uint16_t nDataMask,
290+
bool RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorum& quorum, uint16_t nDataMask,
291291
const uint256& proTxHash = uint256()) const;
292292

293293
// all these methods will lock cs_main for a short period of time
@@ -316,10 +316,10 @@ class CQuorumManager
316316
/// Returns the start offset for the masternode with the given proTxHash. This offset is applied when picking data recovery members of a quorum's
317317
/// memberlist and is calculated based on a list of all member of all active quorums for the given llmqType in a way that each member
318318
/// should receive the same number of request if all active llmqType members requests data from one llmqType quorum.
319-
size_t GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, const CBlockIndex* pIndex) const;
319+
size_t GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const;
320320

321-
void StartCachePopulatorThread(const CQuorumCPtr pQuorum) const;
322-
void StartQuorumDataRecoveryThread(CConnman& connman, const CQuorumCPtr pQuorum, const CBlockIndex* pIndex,
321+
void StartCachePopulatorThread(CQuorumCPtr pQuorum) const;
322+
void StartQuorumDataRecoveryThread(CConnman& connman, CQuorumCPtr pQuorum, const CBlockIndex* pIndex,
323323
uint16_t nDataMask) const;
324324

325325
void StartCleanupOldQuorumDataThread(const CBlockIndex* pIndex) const;

0 commit comments

Comments
 (0)