perf: versionbits avoid calculation which is not used#6632
Conversation
WalkthroughThe change modifies the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Result of `calculateStartHeight` is not always used but it's always calculated
2f74b8d perf: reuse result of BuildNewListFromBlock (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Method `BuildNewListFromBlock` is called twice for each block during validation. These calculation are pretty heavy assuming that there's thousands of masternodes. It is not much for 1 block but that's a lot of CPU calculations during full re-index or initial indexation for 2M of blocks. ## What was done? Methods `CalcCbTxMerkleRootMNList` and `CDeterministicMNManager::ProcessBlock` during block calculation re-use same pre-calculated SML instead internal re-calculation. ## How Has This Been Tested? Benchmarks are done on branch including this changes: #6632 ``` $ getblockcount 2256425 $ getblockhash 2250000 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 $ debug bench $ invalidateblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 null $ reconsiderblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 ``` I run it several times in different days and results are differ, but it is consistently faster. There's logs for 3 different runs, see sum of `m_dmnman` and `CheckCbTxMerkleRoots`. Run time for ~5000blocks: ``` <PR+patches> 2025-04-16T19:31:08Z [bench] - m_dmnman: 1.16ms [5.80s] <--- 2025-04-16T19:31:08Z [bench] - GetTxPayload: 0.22ms [1.38s] 2025-04-16T19:31:08Z [bench] - CalcCbTxMerkleRootMNList: 0.22ms [1.77s] 2025-04-16T19:31:08Z [bench] - CachedGetQcHashesQcIndexedHashes: 0.65ms [5.90s] 2025-04-16T19:31:08Z [bench] - Loop: 0.01ms [0.72s] 2025-04-16T19:31:08Z [bench] - ComputeMerkleRoot: 0.05ms [0.31s] 2025-04-16T19:31:08Z [bench] - CalcCbTxMerkleRootQuorums: 0.73ms [7.01s] 2025-04-16T19:31:08Z [bench] - CheckCbTxMerkleRoots: 2.39ms [16.84s] <--- 2025-04-16T19:31:08Z [bench] - Connect block: 10.88ms [105.65s (17.59ms/blk)] <develop+patches> 2025-04-16T20:02:37Z [bench] - m_dmnman: 1.12ms [6.38s] <--- 2025-04-16T20:02:37Z [bench] - GetTxPayload: 0.20ms [1.40s] 2025-04-16T20:02:37Z [bench] - BuildNewListFromBlock: 0.33ms [2.97s] 2025-04-16T20:02:37Z [bench] - CSimplifiedMNList: 1.08ms [6.56s] 2025-04-16T20:02:37Z [bench] - CalcCbTxMerkleRootMNList: 1.79ms [11.86s] 2025-04-16T20:02:37Z [bench] - CachedGetQcHashesQcIndexedHashes: 0.71ms [6.02s] 2025-04-16T20:02:37Z [bench] - Loop: 0.02ms [0.74s] 2025-04-16T20:02:37Z [bench] - ComputeMerkleRoot: 0.05ms [0.30s] 2025-04-16T20:02:37Z [bench] - CalcCbTxMerkleRootQuorums: 0.80ms [7.14s] 2025-04-16T20:02:37Z [bench] - CheckCbTxMerkleRoots: 2.80ms [20.46s] <--- 2025-04-16T20:02:37Z [bench] - Connect block: 7.35ms [111.00s (18.45ms/blk)] ``` Run time for 6000 blocks: ``` <PR+patches> - m_dmnman: 2.54ms [6.81s] <--- - GetTxPayload: 0.21ms [1.52s] - CalcCbTxMerkleRootMNList: 0.75ms [2.66s] - CachedGetQcHashesQcIndexedHashes: 1.09ms [7.06s] - Loop: 0.02ms [0.73s] - ComputeMerkleRoot: 0.05ms [0.30s] - CalcCbTxMerkleRootQuorums: 1.18ms [8.10s] - CheckCbTxMerkleRoots: 4.54ms [21.03s] <--- ... - Connect block: 14.29ms [113.38s (17.61ms/blk)] <develop+patches> - m_dmnman: 2.17ms [6.64s] <--- - GetTxPayload: 0.24ms [1.45s] - BuildNewListFromBlock: 0.61ms [3.05s] - CSimplifiedMNList: 2.78ms [7.95s] - CalcCbTxMerkleRootMNList: 3.87ms [13.83s] - CachedGetQcHashesQcIndexedHashes: 0.69ms [6.80s] - Loop: 0.02ms [0.71s] - ComputeMerkleRoot: 0.05ms [0.29s] - CalcCbTxMerkleRootQuorums: 0.76ms [7.81s] - CheckCbTxMerkleRoots: 4.89ms [23.09s] <--- ... - Connect block: 16.90ms [114.69s (17.80ms/blk)] ``` Run time for 9000 blocks: ``` <PR+patches> - m_dmnman: 1.96ms [10.34s] <--- - GetTxPayload: 0.23ms [1.98s] - CalcCbTxMerkleRootMNList: 0.41ms [3.75s] - CachedGetQcHashesQcIndexedHashes: 0.81ms [10.01s] - Loop: 0.02ms [0.96s] - ComputeMerkleRoot: 0.04ms [0.40s] - CalcCbTxMerkleRootQuorums: 0.89ms [11.39s] - CheckCbTxMerkleRoots: 3.48ms [30.06s] <--- ... - Connect block: 15.21ms [152.76s (18.04ms/blk)] <develop+patches> - m_dmnman: 2.04ms [11.14s] <--- - GetTxPayload: 0.20ms [2.00s] - BuildNewListFromBlock: 1.09ms [4.23s] - CSimplifiedMNList: 3.08ms [13.82s] - CalcCbTxMerkleRootMNList: 4.88ms [24.59s] - CachedGetQcHashesQcIndexedHashes: 0.87ms [10.22s] - Loop: 0.01ms [0.93s] - ComputeMerkleRoot: 0.05ms [0.39s] - CalcCbTxMerkleRootQuorums: 0.94ms [11.56s] - CheckCbTxMerkleRoots: 6.03ms [38.16s] <--- ... - Connect block: 24.45ms [157.15s (18.55ms/blk)] ``` ## 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 _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: light ACK 2f74b8d PastaPastaPasta: utACK 2f74b8d Tree-SHA512: 61a69501ff096bfdc903abd0ca79a6aeb98b6e71ddde5773639914586232c869d1588471c7232ee10753f2a8bd044668928ce0272dfc569646412525e12cae48
…used b622989 perf: optimize version bit calculation (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It seems as there's O(N^2) for some certain situations when calling `invalidateblock`. See perf result during adding / removing blocks to chain: <img width="687" alt="image" src="https://github.com/user-attachments/assets/472e019e-905b-49b6-9786-aa7343eb5724" /> ## What was done? Call `calculateStartHeight` only if its result is going to be used. ## How Has This Been Tested? ``` $ getblockcount 2255311 $ getblockhash 2250000 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 $ invalidateblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 null ``` With changes in PR it takes 1minute, without them - 3 minutes. <img width="687" alt="image" src="https://github.com/user-attachments/assets/967cb943-429d-405d-8aa2-cb192e0d8317" /> Mainnet re-indexed with no issues. ## Breaking Changes N/A ## Checklist: - [x] 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: light ACK b622989 PastaPastaPasta: utACK b622989 Tree-SHA512: a165b96f8f1a1e2efa64e223639822c9690c0cfbc3c185925826b62ef9a2e92adb4e5913f6c904363fe2ef19e384674d4ab16f082c2a4b9c574ec4ea63747ce9
9ec8e66 docs: add v22.1.3 release notes and archive v22.1.2 (pasta) 95c2a0e chore: bump version to v22.1.3 (pasta) 3345046 chore: update chainparams for v22.1.3 release (pasta) 1db850a Merge #6632: perf: versionbits avoid calculation which is not used (pasta) 4173e3c Merge #6739: docs: Fix broken links from translation instructions and a typo (pasta) 9eda785 Merge #6740: feat: improve logging of non-existing destinations for masternode payment (pasta) 47b6826 Merge #6744: fix: remove useless but alarming log record about spent information (pasta) Pull request description: ## Issue being fixed or feature implemented Backports for a new version, v22.1.3 ## What was done? See release notes ## How Has This Been Tested? ## Breaking Changes None ## Checklist: - [x] 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 9ec8e66 kwvg: utACK 9ec8e66 Tree-SHA512: 9e94b472c32f87c46d60fe53bcec59cb2ec1823e56cfbc2fe4a07d476e1c576c0d7f0c54115aeca902767651591556d68c8e81b81ac9bd12bc5291fa17f5a768
9ec8e66 docs: add v22.1.3 release notes and archive v22.1.2 (pasta) 95c2a0e chore: bump version to v22.1.3 (pasta) 3345046 chore: update chainparams for v22.1.3 release (pasta) 1db850a Merge #6632: perf: versionbits avoid calculation which is not used (pasta) 4173e3c Merge #6739: docs: Fix broken links from translation instructions and a typo (pasta) 9eda785 Merge #6740: feat: improve logging of non-existing destinations for masternode payment (pasta) 47b6826 Merge #6744: fix: remove useless but alarming log record about spent information (pasta) Pull request description: ## Issue being fixed or feature implemented ## What was done? ## How Has This Been Tested? ## Breaking Changes n/a ## Checklist: - [ ] 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: PastaPastaPasta: utACK 2527ca5 kwvg: utACK 2527ca5 Tree-SHA512: 3ecf4f26a816db6be797208ce5d519b8c3571a4aae832d8a1dcf41add4cc48d102459302e1777786b19f577c0a3222d77ce490e2301e73963d6021e2cdc2ad86
2f74b8d perf: reuse result of BuildNewListFromBlock (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Method `BuildNewListFromBlock` is called twice for each block during validation. These calculation are pretty heavy assuming that there's thousands of masternodes. It is not much for 1 block but that's a lot of CPU calculations during full re-index or initial indexation for 2M of blocks. ## What was done? Methods `CalcCbTxMerkleRootMNList` and `CDeterministicMNManager::ProcessBlock` during block calculation re-use same pre-calculated SML instead internal re-calculation. ## How Has This Been Tested? Benchmarks are done on branch including this changes: dashpay#6632 ``` $ getblockcount 2256425 $ getblockhash 2250000 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 $ debug bench $ invalidateblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 null $ reconsiderblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0 ``` I run it several times in different days and results are differ, but it is consistently faster. There's logs for 3 different runs, see sum of `m_dmnman` and `CheckCbTxMerkleRoots`. Run time for ~5000blocks: ``` <PR+patches> 2025-04-16T19:31:08Z [bench] - m_dmnman: 1.16ms [5.80s] <--- 2025-04-16T19:31:08Z [bench] - GetTxPayload: 0.22ms [1.38s] 2025-04-16T19:31:08Z [bench] - CalcCbTxMerkleRootMNList: 0.22ms [1.77s] 2025-04-16T19:31:08Z [bench] - CachedGetQcHashesQcIndexedHashes: 0.65ms [5.90s] 2025-04-16T19:31:08Z [bench] - Loop: 0.01ms [0.72s] 2025-04-16T19:31:08Z [bench] - ComputeMerkleRoot: 0.05ms [0.31s] 2025-04-16T19:31:08Z [bench] - CalcCbTxMerkleRootQuorums: 0.73ms [7.01s] 2025-04-16T19:31:08Z [bench] - CheckCbTxMerkleRoots: 2.39ms [16.84s] <--- 2025-04-16T19:31:08Z [bench] - Connect block: 10.88ms [105.65s (17.59ms/blk)] <develop+patches> 2025-04-16T20:02:37Z [bench] - m_dmnman: 1.12ms [6.38s] <--- 2025-04-16T20:02:37Z [bench] - GetTxPayload: 0.20ms [1.40s] 2025-04-16T20:02:37Z [bench] - BuildNewListFromBlock: 0.33ms [2.97s] 2025-04-16T20:02:37Z [bench] - CSimplifiedMNList: 1.08ms [6.56s] 2025-04-16T20:02:37Z [bench] - CalcCbTxMerkleRootMNList: 1.79ms [11.86s] 2025-04-16T20:02:37Z [bench] - CachedGetQcHashesQcIndexedHashes: 0.71ms [6.02s] 2025-04-16T20:02:37Z [bench] - Loop: 0.02ms [0.74s] 2025-04-16T20:02:37Z [bench] - ComputeMerkleRoot: 0.05ms [0.30s] 2025-04-16T20:02:37Z [bench] - CalcCbTxMerkleRootQuorums: 0.80ms [7.14s] 2025-04-16T20:02:37Z [bench] - CheckCbTxMerkleRoots: 2.80ms [20.46s] <--- 2025-04-16T20:02:37Z [bench] - Connect block: 7.35ms [111.00s (18.45ms/blk)] ``` Run time for 6000 blocks: ``` <PR+patches> - m_dmnman: 2.54ms [6.81s] <--- - GetTxPayload: 0.21ms [1.52s] - CalcCbTxMerkleRootMNList: 0.75ms [2.66s] - CachedGetQcHashesQcIndexedHashes: 1.09ms [7.06s] - Loop: 0.02ms [0.73s] - ComputeMerkleRoot: 0.05ms [0.30s] - CalcCbTxMerkleRootQuorums: 1.18ms [8.10s] - CheckCbTxMerkleRoots: 4.54ms [21.03s] <--- ... - Connect block: 14.29ms [113.38s (17.61ms/blk)] <develop+patches> - m_dmnman: 2.17ms [6.64s] <--- - GetTxPayload: 0.24ms [1.45s] - BuildNewListFromBlock: 0.61ms [3.05s] - CSimplifiedMNList: 2.78ms [7.95s] - CalcCbTxMerkleRootMNList: 3.87ms [13.83s] - CachedGetQcHashesQcIndexedHashes: 0.69ms [6.80s] - Loop: 0.02ms [0.71s] - ComputeMerkleRoot: 0.05ms [0.29s] - CalcCbTxMerkleRootQuorums: 0.76ms [7.81s] - CheckCbTxMerkleRoots: 4.89ms [23.09s] <--- ... - Connect block: 16.90ms [114.69s (17.80ms/blk)] ``` Run time for 9000 blocks: ``` <PR+patches> - m_dmnman: 1.96ms [10.34s] <--- - GetTxPayload: 0.23ms [1.98s] - CalcCbTxMerkleRootMNList: 0.41ms [3.75s] - CachedGetQcHashesQcIndexedHashes: 0.81ms [10.01s] - Loop: 0.02ms [0.96s] - ComputeMerkleRoot: 0.04ms [0.40s] - CalcCbTxMerkleRootQuorums: 0.89ms [11.39s] - CheckCbTxMerkleRoots: 3.48ms [30.06s] <--- ... - Connect block: 15.21ms [152.76s (18.04ms/blk)] <develop+patches> - m_dmnman: 2.04ms [11.14s] <--- - GetTxPayload: 0.20ms [2.00s] - BuildNewListFromBlock: 1.09ms [4.23s] - CSimplifiedMNList: 3.08ms [13.82s] - CalcCbTxMerkleRootMNList: 4.88ms [24.59s] - CachedGetQcHashesQcIndexedHashes: 0.87ms [10.22s] - Loop: 0.01ms [0.93s] - ComputeMerkleRoot: 0.05ms [0.39s] - CalcCbTxMerkleRootQuorums: 0.94ms [11.56s] - CheckCbTxMerkleRoots: 6.03ms [38.16s] <--- ... - Connect block: 24.45ms [157.15s (18.55ms/blk)] ``` ## 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 _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: light ACK 2f74b8d PastaPastaPasta: utACK 2f74b8d Tree-SHA512: 61a69501ff096bfdc903abd0ca79a6aeb98b6e71ddde5773639914586232c869d1588471c7232ee10753f2a8bd044668928ce0272dfc569646412525e12cae48
Issue being fixed or feature implemented
It seems as there's O(N^2) for some certain situations when calling
invalidateblock.See perf result during adding / removing blocks to chain:

What was done?
Call
calculateStartHeightonly if its result is going to be used.How Has This Been Tested?
With changes in PR it takes 1minute, without them - 3 minutes.

Mainnet re-indexed with no issues.
Breaking Changes
N/A
Checklist: