Skip to content
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

partial #15842, merge #15849, #17342, #18710: Add local thread pool to CCheckQueue #4191

Merged
merged 4 commits into from
Jun 26, 2021

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Jun 5, 2021

Overview

This pull request presupposes the merger of #4190 into develop. Only the last four commits are meant to be reviewed. This pull request adds a local thread pool to CCheckQueue in preparation for more threading refactors. To actually compare checkqueue with tlocks, go here

Motivation

During initial preparation for bitcoin#18242 ("Add BIP324 encrypted p2p transport de-/serializer" by jonasschnelli) and the backporting of bitcoin@78c312c, a difference in class and function names were noticed and that in turn lead to a series of pull requests the lead us to this one.

The motivation is to reduce future merge conflicts when backporting pull requests from Bitcoin Core.

Contents

bitcoin#15849, bitcoin#17342, bitcoin#18710

Partials

bitcoin#15842

Disclosures

  • This is a work in progress and needs external review. Dash-specific changes have not been tested, only compilation and unit test success have been ensured.
  • Contains squashed patches b87b772, c6ced95, 589ffcf and b071fe0 by UdjinM6

@PastaPastaPasta PastaPastaPasta marked this pull request as draft June 5, 2021 17:30
@kwvg kwvg marked this pull request as ready for review June 6, 2021 10:58
@kwvg kwvg marked this pull request as draft June 6, 2021 16:35
@kwvg kwvg force-pushed the checkqueue branch 5 times, most recently from 2759ad0 to 3471350 Compare June 10, 2021 09:07
@kwvg kwvg marked this pull request as ready for review June 11, 2021 12:41
@UdjinM6
Copy link

UdjinM6 commented Jun 15, 2021

@kwvg kwvg force-pushed the checkqueue branch 2 times, most recently from d27e546 to 972af32 Compare June 16, 2021 13:03
@UdjinM6 UdjinM6 added this to the 17.1 milestone Jun 16, 2021
@UdjinM6
Copy link

UdjinM6 commented Jun 16, 2021

Looking at the diff now I think I messed up in my fixes a bit 🙈 , pls see UdjinM6@b071fe0

@UdjinM6
Copy link

UdjinM6 commented Jun 24, 2021

LGTM, pls squash c773696 into 187f473 (pr 15849)

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, besides udjin squash and util stuff will require #4197 to be updated

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly tested ACK

@UdjinM6 UdjinM6 merged commit c5cc285 into dashpay:develop Jun 26, 2021
@kwvg kwvg deleted the checkqueue branch July 18, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants