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

Skip GPM and other checks for transactions whose gas limit exceeds what's left it the block #1490

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

oneeman
Copy link
Contributor

@oneeman oneeman commented Apr 8, 2021

Description

This PR implements an optimization to remove a large inefficiency in block creation. The worker goes through, trying to add the transactions in order of decreasing gas price, until it . If it encounters a transaction that requires more gas than is left in the block, it would try to process it and give an error core.ErrGasLimitReached which is then handled by skipping the transaction and continuing trying others (since there may be other transactions later on which require less gas and could fit). The problem was that before this error is returned it does a bunch of work, including core contract calls. This meant that going through transactions which aren't included was taking a long time when the transaction pool had 1000 or more pending transactions.

With this PR, we first check whether the block has enough gas left for the transaction, and if not then we skip right away rather than skipping after getting core.ErrGasLimitReached. Only transactions which would have been skipped anyway are skipped.

This optimization, combined with #1489 and #1467, means the size of the transaction pool will no longer be a bottleneck.

Tested

  • Automated tests
  • Benchmarking on a single validator-network confirmed that even 8192 pending transactions doesn't slow down block creation.
  • Benchmarking on a 100-validator testnet with up to 2000 transactions in the pool confirmed that more transactions in the pool doesn't lead to block creation taking longer as it did before.

Backwards compatibility

No backwards compatibility issues.

@oneeman oneeman requested a review from a team as a code owner April 8, 2021 19:12
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

I checked the short circuit; LGTM

@oneeman oneeman merged commit 1c21d5c into master Apr 8, 2021
@oneeman oneeman deleted the oneeman/mining-optimization branch April 8, 2021 20:23
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