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

Adjust pending nonce update operation #21164

Closed
wants to merge 3 commits into from
Closed

Conversation

WeiLoy
Copy link
Contributor

@WeiLoy WeiLoy commented Jun 3, 2020

  1. In the truncatePending function, the nonce value of the account will be updated immediately when the overflow transaction is deleted, so it is only necessary to update the nonce of all accounts after performing reset to avoid affecting the efficiency of inserting transactions
  2. Update all nonces in pending at one time to avoid frequent lock and unlock when there are too many accounts in pending
  3. Advance the order of updating all nonce actions before truncatePending and truncateQueue so that setIfLower can be effective when updating an account

}
// Ensure pool.queue and pool.pending sizes stay within the configured limits.
pool.truncatePending()
pool.truncateQueue()

// Update all accounts to the latest known pending nonce
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's correct. truncatePending will drop the overflowed transactions from the pending list. In the original code, the pending nonces will be set back.

Copy link
Contributor Author

@WeiLoy WeiLoy Jun 4, 2020

Choose a reason for hiding this comment

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

  1. But in truncatePending, the nonce of the account will be updated through pool.pendingNonces.setIfLower while deleting the overflowed transaction. The account that needs to be updated nonce has been updated in this function, so There is no need to reset the account again outside the function.
  2. In the original code, if you are performing the reset action, first execute truncatePending. Since the value of pool.pendingNonces has been cleared in the reset function, the subsequent steps are executed. When truncatePending deletes the overflow transaction, pool.pendingNonces.setIfLower actually reads the nonce in StateDB to judge, the condition is not established and will not be updated to the highest nonce, and finally by direct update All accounts are realized
  3. So only need to update all nonce to pool.pendingNonces at the time of reset, and after the update is completed, then execute truncatePending, this time if the function deletes the overflow transaction, pool .pendingNonces.setIfLower is able to update the nonce of the corresponding account

@adamschmideg
Copy link
Contributor

Can you write some benchmarks to show this change is worth the risk?

@WeiLoy
Copy link
Contributor Author

WeiLoy commented Jun 16, 2020

This is before optimization:
BenchmarkPoolMultiAccount_old

This is after optimization:
BenchmarkPoolMultiAccount

@holiman
Copy link
Contributor

holiman commented Jan 25, 2021

Closing in favour of #22231 (same as this PR but rebased on master) + some minor fixes

@holiman holiman closed this Jan 25, 2021
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.

5 participants