- 
        Couldn't load subscription status. 
- Fork 21.5k
core/types: reduce allocations for transaction comparison #31912
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/types: reduce allocations for transaction comparison #31912
Conversation
Signed-off-by: Csaba Kiraly <[email protected]>
Signed-off-by: Csaba Kiraly <[email protected]>
ba1d582    to
    8bd1d31      
    Compare
  
    Better check in case this is used as library. Not using MustFromBig as we don't have control over how this is being used. Signed-off-by: Csaba Kiraly <[email protected]>
| @MariusVanDerWijden I was adding overflow checks everywhere in case this is used in some other context. Not using MustFromBig to avod panic in those cases. Let me know if you agree. | 
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
…1912) This PR should reduce overall allocations of a running node by ~10 percent. Since most allocations are coming from the re-heaping of the transaction pool. ``` (pprof) list EffectiveGasTipCmp Total: 38197204475 ROUTINE ======================== github.com/ethereum/go-ethereum/core/types.(*Transaction).EffectiveGasTipCmp in github.com/ethereum/go-ethereum/core/types/transaction.go 0 3766837369 (flat, cum) 9.86% of Total . . 386:func (tx *Transaction) EffectiveGasTipCmp(other *Transaction, baseFee *big.Int) int { . . 387: if baseFee == nil { . . 388: return tx.GasTipCapCmp(other) . . 389: } . . 390: // Use more efficient internal method. . . 391: txTip, otherTip := new(big.Int), new(big.Int) . 1796172553 392: tx.calcEffectiveGasTip(txTip, baseFee) . 1970664816 393: other.calcEffectiveGasTip(otherTip, baseFee) . . 394: return txTip.Cmp(otherTip) . . 395:} . . 396: . . 397:// EffectiveGasTipIntCmp compares the effective gasTipCap of a transaction to the given gasTipCap. . . 398:func (tx *Transaction) EffectiveGasTipIntCmp(other *big.Int, baseFee *big.Int) int { ``` This PR reduces the allocations for comparing two transactions from 2 to 0: ``` goos: linux goarch: amd64 pkg: github.com/ethereum/go-ethereum/core/types cpu: Intel(R) Core(TM) Ultra 7 155U │ /tmp/old.txt │ /tmp/new.txt │ │ sec/op │ sec/op vs base │ EffectiveGasTipCmp/Original-14 64.67n ± 2% 25.13n ± 9% -61.13% (p=0.000 n=10) │ /tmp/old.txt │ /tmp/new.txt │ │ B/op │ B/op vs base │ EffectiveGasTipCmp/Original-14 16.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10) │ /tmp/old.txt │ /tmp/new.txt │ │ allocs/op │ allocs/op vs base │ EffectiveGasTipCmp/Original-14 2.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) ``` It also speeds up the process by ~60% There are two minor caveats with this PR: - We change the API for `EffectiveGasTipCmp` and `EffectiveGasTipIntCmp` (which are probably not used by much) - We slightly change the behavior of `tx.EffectiveGasTip` when it returns an error. It would previously return a negative number on error, now it does not (since uint256 does not allow for negative numbers) --------- Signed-off-by: Csaba Kiraly <[email protected]> Co-authored-by: Csaba Kiraly <[email protected]>
This PR should reduce overall allocations of a running node by ~10 percent. Since most allocations are coming from the re-heaping of the transaction pool.
This PR reduces the allocations for comparing two transactions from 2 to 0:
It also speeds up the process by ~60%
There are two minor caveats with this PR:
EffectiveGasTipCmpandEffectiveGasTipIntCmp(which are probably not used by much)tx.EffectiveGasTipwhen it returns an error. It would previously return a negative number on error, now it does not (since uint256 does not allow for negative numbers)