-
Notifications
You must be signed in to change notification settings - Fork 19.6k
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
core/txpool, miner: speed up blob pool pending retrievals #29008
Conversation
f04a19b
to
a1f4b93
Compare
The previous PR made txs := newTransactionsByPriceAndNonce(env.signer, remoteTxs, baseFee)
if err := w.commitTransactions(env, txs, interrupt, tip); err != nil { In this method, we iterate them all once more, and wrap it as func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address][]*txpool.LazyTransaction, baseFee *uint256.Int) *transactionsByPriceAndNonce {
// Initialize a price and received time based heap with the head transactions
heads := make(txByPriceAndTime, 0, len(txs))
for from, accTxs := range txs {
wrapped, err := newTxWithMinerFee(accTxs[0], from, baseFee) We also split it up
We also do a Now, however, they are fee-aware, so why not make them return something like this // pendingSet represents a set of pending transactions, delivered by a subpool.
type pendingSet{
heads []*LazyTransaction // Next transaction for each unique account (price heap)
txs map[common.Address][]*LazyTransaction// Per account nonce-sorted list of transactions
} We could even push it a bit further, so that heads is not
One final thought, not sure it's worth it:
And, as we've already discussed, having a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds a benchmark to the blobpool to measure the execution speed of pending transaction retrieval. Note, the benchmark uses an in-memory bill of 10GB and needs to pre-generate 80K private keys. It takes 5 minutes.
Based on the benchmark, the PR also contains some fixes to the pending retrieval:
Due to the last one, the PR changes this all over the place. This does mean that the legacy pool starts to pay the price in exchange. That said, the legacy pool is capped to about 4K txs, whereas the blobpool currently 80K, so it's worth it. Also, ideally the legacy pool would also start using uint256 and should be resolved like that.