Merge dashpay#6731: perf: optimize GetQuorumQuarterMembersBySnapshot calculation

b7b04e5 fix: rename dmnman to allMns to match declaration and definition of function; and the meaning (Konstantin Akimov)
e9e0f58 refactor: inline GetMNUsageBySnapshot to simplify code (Konstantin Akimov)
46383b1 perf: optimize GetQuorumQuarterMembersBySnapshot calculation (Konstantin Akimov)
d337579 refactor: use CDeterministicMNList instead CDeterministicMNManager in GetMNUsageBySnapshot (Konstantin Akimov)
790ae0f refactor: use CDeterministicMNList instead of CDeterministicMNManager in ComputeQuorumMembers (Konstantin Akimov)
ac934eb refactor: pass CDeterministicMNList instead of CDeterministicMNManager to BuildNewQuorumQuarterMembers (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Masternodes already sorted with the same modifier inside `GetMNUsageBySnapshot`, no need to re-sort them inside `GetQuorumQuarterMembersBySnapshot`.

  ## What was done?
  Removed extra sort of already sorted vectors; removed duplicated calculation of modifier, simplified `GetMNUsageBySnapshot` by inlining it.
  Also several methods are changed to accept CDeterministicMNList instead of CDeterministicMNManager.

  ## How Has This Been Tested?
  It improves significantly performance of `llmq::utils::GetAllQuorumMembers` which is widely used during block validation; for many RPC calls, for processing some network messages, for masternodes participating in quorums.
  Despite 25% improvement for performance of `llmq::utils::GetAllQuorumMembers` overall improvement for indexing is less than 1% of total CPU time (thanks to caches).

  `perf` for PR:
  <img width="838" alt="image" src="https://github.com/user-attachments/assets/cb98a0ef-3a4b-4d21-a674-80c300764712" />

  `perf` for develop:
  <img width="838" alt="image" src="https://github.com/user-attachments/assets/879acdac-facd-4a11-b37a-607f4cb31e48" />

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] 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
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK b7b04e5

Tree-SHA512: 069e2a0a2b6ad7dfb092fdfa14e289ecde5bc25a59c7cb0c2fad10a56c9dbacb207b8f4bdd5be0eb88380fd745e0ff21ffc88fe5f24103d376cd3dc83f089787

Author: PastaPastaPasta

src/llmq/utils.cpp

@@ -56,14 +56,15 @@ struct PreviousQuorumQuarters {
 };
 
 // Forward declarations
-static std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqType, CDeterministicMNManager& dmnman,
+static std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqType,
+                                                              const CDeterministicMNList& mn_list,
                                                               const CBlockIndex* pQuorumBaseBlockIndex);
 static std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRotation(
     const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, CQuorumSnapshotManager& qsnapman,
     const CBlockIndex* pCycleQuorumBaseBlockIndex);
 
 static std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(
-    const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, CQuorumSnapshotManager& qsnapman,
+    const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns, CQuorumSnapshotManager& qsnapman,
     const CBlockIndex* pCycleQuorumBaseBlockIndex, const PreviousQuorumQuarters& previousQuarters);
 
 static PreviousQuorumQuarters GetPreviousQuorumQuarterMembers(const Consensus::LLMQParams& llmqParams,
@@ -75,9 +76,6 @@ static PreviousQuorumQuarters GetPreviousQuorumQuarterMembers(const Consensus::L
 static std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot(
     const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman,
     const CBlockIndex* pCycleQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeight);
-static std::pair<std::vector<CDeterministicMNCPtr>, std::vector<CDeterministicMNCPtr>> GetMNUsageBySnapshot(
-    const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman,
-    const CBlockIndex* pCycleQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeight);
 
 static void BuildQuorumSnapshot(const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns,
                                 const CDeterministicMNList& mnUsedAtH,
@@ -271,15 +269,19 @@ std::vector<CDeterministicMNCPtr> GetAllQuorumMembers(Consensus::LLMQType llmqTy
                                                       std::move(q[i]));
         }
     } else {
-        quorumMembers = ComputeQuorumMembers(llmqType, dmnman, pQuorumBaseBlockIndex);
+        const CBlockIndex* pWorkBlockIndex = IsV20Active(pQuorumBaseBlockIndex)
+                                                 ? pQuorumBaseBlockIndex->GetAncestor(pQuorumBaseBlockIndex->nHeight - 8)
+                                                 : pQuorumBaseBlockIndex.get();
+        CDeterministicMNList mn_list = dmnman.GetListForBlock(pWorkBlockIndex);
+        quorumMembers = ComputeQuorumMembers(llmqType, mn_list, pQuorumBaseBlockIndex);
     }
 
     LOCK(cs_members);
     mapQuorumMembers[llmqType].insert(pQuorumBaseBlockIndex->GetBlockHash(), quorumMembers);
     return quorumMembers;
 }
 
-std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqType, CDeterministicMNManager& dmnman,
+std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqType, const CDeterministicMNList& mn_list,
                                                        const CBlockIndex* pQuorumBaseBlockIndex)
 {
     bool EvoOnly = (Params().GetConsensus().llmqTypePlatform == llmqType) && IsV19Active(pQuorumBaseBlockIndex);
@@ -290,12 +292,8 @@ std::vector<CDeterministicMNCPtr> ComputeQuorumMembers(Consensus::LLMQType llmqT
         return {};
     }
 
-    const CBlockIndex* pWorkBlockIndex = IsV20Active(pQuorumBaseBlockIndex) ?
-            pQuorumBaseBlockIndex->GetAncestor(pQuorumBaseBlockIndex->nHeight - 8) :
-            pQuorumBaseBlockIndex;
     const auto modifier = GetHashModifier(llmq_params_opt.value(), pQuorumBaseBlockIndex);
-    auto allMns = dmnman.GetListForBlock(pWorkBlockIndex);
-    return CalculateQuorum(allMns, modifier, llmq_params_opt->size, EvoOnly);
+    return CalculateQuorum(mn_list, modifier, llmq_params_opt->size, EvoOnly);
 }
 
 std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRotation(
@@ -314,7 +312,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
     const CBlockIndex* pBlockHMinus2CIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 2 * cycleLength);
     const CBlockIndex* pBlockHMinus3CIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 3 * cycleLength);
     const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);
-    auto allMns = dmnman.GetListForBlock(pWorkBlockIndex);
+    CDeterministicMNList allMns = dmnman.GetListForBlock(pWorkBlockIndex);
     LogPrint(BCLog::LLMQ, "ComputeQuorumMembersByQuarterRotation llmqType[%d] nHeight[%d] allMns[%d]\n", ToUnderlying(llmqType), pCycleQuorumBaseBlockIndex->nHeight, allMns.GetValidMNsCount());
 
     PreviousQuorumQuarters previousQuarters = GetPreviousQuorumQuarterMembers(llmqParams, dmnman, qsnapman,
@@ -325,7 +323,7 @@ std::vector<std::vector<CDeterministicMNCPtr>> ComputeQuorumMembersByQuarterRota
     size_t nQuorums = static_cast<size_t>(llmqParams.signingActiveQuorumCount);
     std::vector<std::vector<CDeterministicMNCPtr>> quorumMembers{nQuorums};
 
-    auto newQuarterMembers = BuildNewQuorumQuarterMembers(llmqParams, dmnman, qsnapman, pCycleQuorumBaseBlockIndex,
+    auto newQuarterMembers = BuildNewQuorumQuarterMembers(llmqParams, allMns, qsnapman, pCycleQuorumBaseBlockIndex,
                                                           previousQuarters);
     //TODO Check if it is triggered from outside (P2P, block validation). Throwing an exception is probably a wiser choice
     //assert (!newQuarterMembers.empty());
@@ -415,7 +413,7 @@ PreviousQuorumQuarters GetPreviousQuorumQuarterMembers(const Consensus::LLMQPara
 }
 
 std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(
-    const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, CQuorumSnapshotManager& qsnapman,
+    const Consensus::LLMQParams& llmqParams, const CDeterministicMNList& allMns, CQuorumSnapshotManager& qsnapman,
     const CBlockIndex* pCycleQuorumBaseBlockIndex, const PreviousQuorumQuarters& previousQuarters)
 {
     if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
@@ -428,10 +426,8 @@ std::vector<std::vector<CDeterministicMNCPtr>> BuildNewQuorumQuarterMembers(
 
     size_t quorumSize = static_cast<size_t>(llmqParams.size);
     auto quarterSize{quorumSize / 4};
-    const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);
     const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);
 
-    auto allMns = dmnman.GetListForBlock(pWorkBlockIndex);
 
     if (allMns.GetValidMNsCount() < quarterSize) {
         return quarterQuorumMembers;
@@ -617,14 +613,28 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
 
     std::vector<CDeterministicMNCPtr> sortedCombinedMns;
     {
+        const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(
+            pCycleQuorumBaseBlockIndex->nHeight - 8);
+        auto mn_list = dmnman.GetListForBlock(pWorkBlockIndex);
         const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);
-        auto [MnsUsedAtH, MnsNotUsedAtH] = GetMNUsageBySnapshot(llmqParams, dmnman, pCycleQuorumBaseBlockIndex,
-                                                                snapshot, nHeight);
-        // the list begins with all the unused MNs
-        sortedCombinedMns = CalculateQuorum(std::move(MnsNotUsedAtH), modifier);
+        auto sortedAllMns = CalculateQuorum(mn_list, modifier);
+
+        std::vector<CDeterministicMNCPtr> usedMNs;
+        size_t i{0};
+        for (const auto& dmn : sortedAllMns) {
+            if (snapshot.activeQuorumMembers[i]) {
+                usedMNs.push_back(dmn);
+            } else {
+                if (!dmn->pdmnState->IsBanned()) {
+                    // the list begins with all the unused MNs
+                    sortedCombinedMns.push_back(dmn);
+                }
+            }
+            i++;
+        }
+
         // Now add the already used MNs to the end of the list
-        auto sortedMnsUsedAtH = CalculateQuorum(std::move(MnsUsedAtH), modifier);
-        std::move(sortedMnsUsedAtH.begin(), sortedMnsUsedAtH.end(), std::back_inserter(sortedCombinedMns));
+        std::move(usedMNs.begin(), usedMNs.end(), std::back_inserter(sortedCombinedMns));
     }
 
     if (LogAcceptDebug(BCLog::LLMQ)) {
@@ -700,39 +710,6 @@ std::vector<std::vector<CDeterministicMNCPtr>> GetQuorumQuarterMembersBySnapshot
     }
 }
 
-static std::pair<std::vector<CDeterministicMNCPtr>, std::vector<CDeterministicMNCPtr>> GetMNUsageBySnapshot(
-    const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman,
-    const CBlockIndex* pCycleQuorumBaseBlockIndex, const llmq::CQuorumSnapshot& snapshot, int nHeight)
-{
-    if (!llmqParams.useRotation || pCycleQuorumBaseBlockIndex->nHeight % llmqParams.dkgInterval != 0) {
-        ASSERT_IF_DEBUG(false);
-        return {};
-    }
-
-    std::vector<CDeterministicMNCPtr> usedMNs;
-    std::vector<CDeterministicMNCPtr> nonUsedMNs;
-
-    const CBlockIndex* pWorkBlockIndex = pCycleQuorumBaseBlockIndex->GetAncestor(pCycleQuorumBaseBlockIndex->nHeight - 8);
-    const auto modifier = GetHashModifier(llmqParams, pCycleQuorumBaseBlockIndex);
-
-    auto allMns = dmnman.GetListForBlock(pWorkBlockIndex);
-    auto sortedAllMns = CalculateQuorum(allMns, modifier);
-
-    size_t i{0};
-    for (const auto& dmn : sortedAllMns) {
-        if (snapshot.activeQuorumMembers[i]) {
-            usedMNs.push_back(dmn);
-        } else {
-            if (!dmn->pdmnState->IsBanned()) {
-                nonUsedMNs.push_back(dmn);
-            }
-        }
-        i++;
-    }
-
-    return {usedMNs, nonUsedMNs};
-}
-
 uint256 DeterministicOutboundConnection(const uint256& proTxHash1, const uint256& proTxHash2)
 {
     // We need to deterministically select who is going to initiate the connection. The naive way would be to simply