-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: reduce needs on cs_main in network processing #6990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
refactor: reduce needs on cs_main in network processing #6990
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it be atomic?
also fSyncStarted; nBlocksInFlight, fPreferredDownload, fPreferHeaders', fPreferHeadersCmpressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but need to be more careful that full operation is atomic. I figured just using a mutex currently would be simplier
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.