Skip to content

core/types: reduce allocations in effectiveGasTip #31602

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MariusVanDerWijden
Copy link
Member

Reduces unnecessary allocations in (types.Transaction).EffectiveGasTip.
Halves the allocations, increases speed by 47%

ROUTINE ======================== github.com/ethereum/go-ethereum/core/types.(*Transaction).EffectiveGasTip in github.com/ethereum/go-ethereum/core/types/transaction.go
     4.40s    370.90s (flat, cum)  1.61% of Total
     480ms      480ms    357:func (tx *Transaction) EffectiveGasTip(baseFee *big.Int) (*big.Int, error) {
     450ms      450ms    358:	if baseFee == nil {
         .          .    359:		return tx.GasTipCap(), nil
         .          .    360:	}
         .          .    361:	var err error
     350ms    232.73s    362:	gasFeeCap := tx.GasFeeCap()
     430ms      8.51s    363:	if gasFeeCap.Cmp(baseFee) < 0 {
     180ms      180ms    364:		err = ErrGasFeeCapTooLow
         .          .    365:	}
     290ms     26.02s    366:	gasFeeCap = gasFeeCap.Sub(gasFeeCap, baseFee)
         .          .    367:
     200ms     95.63s    368:	gasTipCap := tx.GasTipCap()
     680ms      5.56s    369:	if gasTipCap.Cmp(gasFeeCap) < 0 {
     250ms      250ms    370:		return gasTipCap, err
         .          .    371:	}
     1.09s      1.09s    372:	return gasFeeCap, err
         .          .    373:}

Benchmark:

func BenchmarkEffectiveTip(b *testing.B) {
	to := common.Address{}
	tx := NewTx(&DynamicFeeTx{
		ChainID:   big.NewInt(123),
		Nonce:     1,
		Gas:       1000000,
		To:        &to,
		Value:     big.NewInt(1),
		GasTipCap: big.NewInt(500),
		GasFeeCap: big.NewInt(500),
	})
	basefee := big.NewInt(12)
	for i := 0; i < b.N; i++ {
		tx.EffectiveGasTip(basefee)
	}
}
                │ /tmp/old.txt │            /tmp/new.txt             │
                │    sec/op    │    sec/op     vs base               │
EffectiveTip-14   97.83n ± ∞ ¹   57.26n ± ∞ ¹  -41.47% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95

                │ /tmp/old.txt │            /tmp/new.txt            │
                │     B/op     │    B/op      vs base               │
EffectiveTip-14    80.00 ± ∞ ¹   40.00 ± ∞ ¹  -50.00% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95

                │ /tmp/old.txt │            /tmp/new.txt            │
                │  allocs/op   │  allocs/op   vs base               │
EffectiveTip-14    4.000 ± ∞ ¹   2.000 ± ∞ ¹  -50.00% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95

@MariusVanDerWijden
Copy link
Member Author

NVM I have a better idea...

@MariusVanDerWijden
Copy link
Member Author

                │ /tmp/old.txt │            /tmp/new.txt             │
                │    sec/op    │   sec/op     vs base                │
EffectiveTip-14    211.6n ± 1%   111.8n ± 5%  -47.19% (p=0.000 n=10)

                │ /tmp/old.txt │            /tmp/new.txt            │
                │     B/op     │    B/op     vs base                │
EffectiveTip-14    160.00 ± 0%   80.00 ± 0%  -50.00% (p=0.000 n=10)

                │ /tmp/old.txt │            /tmp/new.txt            │
                │  allocs/op   │ allocs/op   vs base                │
EffectiveTip-14     8.000 ± 0%   4.000 ± 0%  -50.00% (p=0.000 n=10)

if gasFeeCap.Cmp(baseFee) < 0 {
err = ErrGasFeeCapTooLow
}
gasFeeCap = gasFeeCap.Sub(gasFeeCap, baseFee)
gasFeeCap = new(big.Int).Sub(gasFeeCap, baseFee)
Copy link
Member Author

Choose a reason for hiding this comment

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

I would love to get rid of this allocation, but I can't

@fjl
Copy link
Contributor

fjl commented Apr 10, 2025

One thing to keep in mind with these optimizations, is that the TxData methods (like gasTipCap) do not always return copies. So the rule is basically that anything returned by them must be copied in the Transaction accessor before returning.

There is also a PR with an alternative approach here: #31598

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.

2 participants