Skip to content

miner: skip loop processing and allocations for empty transaction events#2285

Open
shinobi133 wants to merge 4 commits into
0xPolygon:developfrom
shinobi133:fix/miner-performance-opt
Open

miner: skip loop processing and allocations for empty transaction events#2285
shinobi133 wants to merge 4 commits into
0xPolygon:developfrom
shinobi133:fix/miner-performance-opt

Conversation

@shinobi133

Copy link
Copy Markdown

Summary

<one paragraph: what changed and why. If the change has measurable impact, add a small table or numbers (e.g. perf delta on a kurtosis devnet, mutation kill rate). Link the JIRA / GH issue inline if there is one — not required.>

Executed tests

<what was actually run beyond CI's standard unit / integration / e2e gates: kurtosis scenarios, chaos runs, manual checks against Amoy / mainnet RPCs, devnet upgrades, etc. Include output or pointers to where the run lives.>

Rollout notes

<consensus-affecting? requires coordinated upgrade? backwards-compatible? operator-facing change?>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the miner worker’s transaction-event handling path in mainLoop to avoid doing work when a NewTxsEvent contains no transactions, aiming to reduce unnecessary processing/allocations in that case.

Changes:

  • Adds an early exit when ev.Txs is empty.
  • Introduces a parallel “sender cache warm-up” step for non-empty transaction batches before grouping transactions by sender.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread miner/worker.go Outdated
Comment on lines +931 to +948
if len(ev.Txs) == 0 {
w.newTxs.Add(0)
continue
}

// Warm up cryptographic sender caches in parallel across cores
var wg sync.WaitGroup
for _, tx := range ev.Txs {
wg.Add(1)
go func(t *types.Transaction) {
defer wg.Done()
if w.current != nil {
types.Sender(w.current.signer, t)
}
}(tx)
}
wg.Wait()

@pratikspatil024

Copy link
Copy Markdown
Member

@shinobi133 - please fix the CI and address the comments

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread miner/worker.go Outdated
Comment on lines +931 to +949
if len(ev.Txs) == 0 {
w.newTxs.Add(0)
continue
}

txs := make(map[common.Address][]*txpool.LazyTransaction)
for _, tx := range ev.Txs {
acc, _ := types.Sender(w.current.signer, tx)
txs[acc] = append(txs[acc], &txpool.LazyTransaction{
Pool: w.eth.TxPool(),
Hash: tx.Hash(),
Tx: tx,
Time: tx.Time(),
GasFeeCap: uint256.MustFromBig(tx.GasFeeCap()),
GasTipCap: uint256.MustFromBig(tx.GasTipCap()),
Gas: tx.Gas(),
BlobGas: tx.BlobGas(),
})
}
@sonarqubecloud

Copy link
Copy Markdown

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