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

Mempool: Improve mempool's concurrency #7145

Closed
wants to merge 4 commits into from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Dec 1, 2015

This improves concurrency by not locking CTxMemPool every time CBlockPolicyEstimator is used (although as said this is incomplete and the concurrency improvements can be continued), so it may have a positive impact on performance. But I haven't done any benchmark or test.

My original plan was to do this at once while encapsulating the global minRelayTxFee, but relay policy encapsulation is not welcomed at this point and now it would be more disruptive to completely decouple CTxMemPool from CBlockPolicyEstimator than it used to be when first coded this many months ago.
We can take the first steps instead of waiting to do it at once, and that would hopefully prevent the code from evolving to something where is even harder to make this happen.
Although @morcos thinks that CBlockPolicyEstimator should depend on CTxMemPool and I disagree on that, we both agree that CTxMemPool should not depend on CBlockPolicyEstimator. This PR reduces CTxMemPool's dependency on CBlockPolicyEstimator.

@dcousens
Copy link
Contributor

dcousens commented Dec 2, 2015

@jtimon any reason for close?

@jtimon
Copy link
Contributor Author

jtimon commented Dec 2, 2015

It is incomplete and I know from past experience that it will be very expensive to maintain if not merged fast.
I'm happy to complete and reopen it if/when there's interest in doing this, but it didn't seem to be the case at this point from the reactions I got from the few people I asked for review.
I will complete it in my own personal branch on top of 0.12 once it is released, and then periodically forward-port that branch to 0.13, etc. I will always be happy to upstream things from that branch to Bitcoin Core, but there's no reason for me to keep rebasing disruptive patches on top of the moving target that is bitcoin/master and wasting review time unless I believe they have reasonable chances of getting merged while opened. Nothing seems to indicate that is the case now.

@dcousens
Copy link
Contributor

dcousens commented Dec 2, 2015

No worries 👍, thanks for the response. Indeed bitcoin/bitcoin is fast moving lately.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants