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

[DRAFT] 10x improvement to pop_blocks operation with flag denoting txis are already verified #9148

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0xFFFC0000
Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 commented Feb 1, 2024

Add already_verified flag to add transaction operation of the txpool. This is very useful when we are popping block, and txis are already verified in that situation, which prevents verifying transactions again.

Numbers on my machine:

popping 500 blocks: master branch ~50 seconds, with this PR ~6 seconds.
popping 2000 blocks: master branch ~210 seconds, with this PR ~29 seconds.

Profiling information of monerod on master branch. As you can see, 80-90% of computation is happening in check_tx_inputs function. The profiling output is attached to this message.

monerod-master-branch-pop_blocks-profile.pdf

@0xFFFC0000 0xFFFC0000 marked this pull request as ready for review February 1, 2024 16:44
@0xFFFC0000 0xFFFC0000 changed the title [DRAFT] 10x improvement to pop_blocks operation with flag denoting txis are already verified Feb 1, 2024
@0xFFFC0000 0xFFFC0000 marked this pull request as draft February 1, 2024 17:12
@0xFFFC0000 0xFFFC0000 changed the title 10x improvement to pop_blocks operation with flag denoting txis are already verified Feb 1, 2024
@jeffro256
Copy link
Contributor

FWIW, I wrote a similar change in PR #9135: the caller can provide a hard fork version for which it is known that the transaction passes non-input consensus checks. If that is the same version as the version passed into add_tx, then it skips the non-input consensus checks. This is a safer and clearer code path IMO. Also, skipping the input consensus checks is incorrect behavior for txs not kept by block, since a transaction can verify input checks once but then the block reorgs.

This is very useful when we are popping block, and txis are already
verified in that situation, which prevents verifying transactions again.
@vtnerd
Copy link
Contributor

vtnerd commented Feb 5, 2024

I would definitely hold off on working too hard on this until @jeffro256 gets his changes in - even if there is additional time savings beyond his PR. He has a pretty big change coming soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment