refactor: reduce needs on cs_main in network processing#6990
refactor: reduce needs on cs_main in network processing#6990PastaPastaPasta wants to merge 2 commits intodashpay:developfrom
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
7030342 to
3a8b312
Compare
|
This pull request has conflicts, please rebase. |
12f5253 to
96ff7be
Compare
| //! Length of current-streak of unconnecting headers announcements | ||
| int nUnconnectingHeaders{0}; | ||
| int nUnconnectingHeaders GUARDED_BY(m_mutex){0}; |
There was a problem hiding this comment.
can it be atomic?
also fSyncStarted; nBlocksInFlight, fPreferredDownload, fPreferHeaders', fPreferHeadersCmpressed?
There was a problem hiding this comment.
Maybe, but need to be more careful that full operation is atomic. I figured just using a mutex currently would be simplier
|
This pull request has conflicts, please rebase. |
Issue being fixed or feature implemented
see: #6953 (comment) -- based on that collected data; the cs_main contention in network processing is ~17% of cs_main contention. If we exclude the islock cs_main (resolved (or mostly resolved don't recall) in that PR) the percentage grows to 30%; if we can minimize contention here, it will benefit our overall cs_main contention data.
What was done?
convert CConman to a structure that is more in line with peerman; namely, objects protect their own internal data, and the vector stores shared pointers, so that we take a shared_ptr from the vector, and then can use it as we desire without lifetime concerns.
Be aware, I would like to upstream these changes, so that we don't have to maintain them long term, however there's no guarantee that will be viable.
How Has This Been Tested?
Deployed testing was minimal; after 6953 is merged; I'd like to deploy this version to some testnet nodes and analyze contention.
Breaking Changes
None
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.