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.

Sign up for GitHub

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

Jump to bottom

transaction pool optimizations #21328

Merged
merged 3 commits into from
Jul 14, 2020
Merged

transaction pool optimizations #21328

merged 3 commits into from
Jul 14, 2020

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Jul 13, 2020

This PR contains some of the commits from the (now reverted) #21232 , leaving out the erroneous conversions to uint64

[user@work core]$ benchstat pool.before pool.after 
name                         old time/op    new time/op    delta
PoolBatchInsert100-6           8.52ms ± 2%    8.50ms ± 2%     ~     (p=1.000 n=5+5)
PoolBatchInsert1000-6          99.2ms ± 1%   101.3ms ± 2%     ~     (p=0.190 n=4+5)
PoolBatchInsert10000-6          1.17s ± 9%     1.17s ±12%     ~     (p=1.000 n=5+5)
PoolBatchLocalInsert100-6       350ms ± 5%      31ms ± 3%  -91.20%  (p=0.008 n=5+5)
PoolBatchLocalInsert1000-6      5.15s ± 8%     0.40s ± 2%  -92.19%  (p=0.016 n=5+4)
PoolBatchLocalInsert10000-6     36.5s ± 2%      3.0s ± 4%  -91.84%  (p=0.036 n=3+5)

name                         old alloc/op   new alloc/op   delta
PoolBatchInsert100-6            293kB ± 0%     264kB ± 0%   -9.71%  (p=0.008 n=5+5)
PoolBatchInsert1000-6          2.63MB ± 0%    2.60MB ± 0%   -1.31%  (p=0.016 n=5+4)
PoolBatchInsert10000-6         22.7MB ± 0%    22.7MB ± 0%   -0.13%  (p=0.008 n=5+5)
PoolBatchLocalInsert100-6      14.7MB ± 0%     0.3MB ± 0%  -97.80%  (p=0.008 n=5+5)
PoolBatchLocalInsert1000-6      205MB ± 8%       3MB ± 0%  -98.71%  (p=0.016 n=5+4)
PoolBatchLocalInsert10000-6    1.46GB ± 0%    0.03GB ± 0%  -98.17%  (p=0.036 n=3+5)

name                         old allocs/op  new allocs/op  delta
PoolBatchInsert100-6            3.78k ± 0%     3.78k ± 0%     ~     (p=0.190 n=5+5)
PoolBatchInsert1000-6           37.8k ± 0%     37.8k ± 0%   -0.16%  (p=0.000 n=5+4)
PoolBatchInsert10000-6           339k ± 0%      339k ± 0%     ~     (p=0.548 n=5+5)
PoolBatchLocalInsert100-6       4.54k ± 0%     3.90k ± 0%  -14.15%  (p=0.016 n=5+4)
PoolBatchLocalInsert1000-6      46.2k ± 1%     38.2k ± 0%  -17.19%  (p=0.008 n=5+5)
PoolBatchLocalInsert10000-6      445k ± 0%      381k ± 0%  -14.36%  (p=0.036 n=3+5)

@holiman holiman changed the title Txfixes transaction pool optimizations Jul 13, 2020
MariusVanDerWijden
MariusVanDerWijden approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

LGTM

ucwong
ucwong reviewed Jul 14, 2020
View reviewed changes
core/tx_pool.go
txs := list.Flatten() // Heavy but will be cached and is needed by the miner anyway
pool.pendingNonces.set(addr, txs[len(txs)-1].Nonce()+1)
highestPending := list.LastElement()
pool.pendingNonces.set(addr, highestPending.Nonce()+1)
Copy link
Contributor

@ucwong ucwong Jul 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Why not use one line

pool.pendingNonces.set(addr, list.LastElement().Nonce()+1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be fine too. As it is, it's more obvious for a reader that we're getting the highest nonce:d transaction, so it's a bit simpler to understand what's happening. I don't see any compelling reason to change it now

@holiman holiman added this to the 1.9.17 milestone Jul 14, 2020
@holiman holiman merged commit 6c9f040 into ethereum:master Jul 14, 2020
@ricardolyn ricardolyn mentioned this pull request Feb 19, 2021
Merged
9 tasks
enriquefynn pushed a commit to enriquefynn/go-ethereum that referenced this pull request Mar 10, 2021
@holiman @MariusVanDerWijden @enriquefynn
core: transaction pool optimizations (ethereum#21328)
9af93ef 
* core: added local tx pool test case

* core, crypto: various allocation savings regarding tx handling

* core/txlist, txpool: save a reheap operation, avoid some bigint allocs

Co-authored-by: Marius van der Wijden <[email protected]>
@gzliudan gzliudan mentioned this pull request May 13, 2024
Merged
20 tasks
gzliudan pushed a commit to gzliudan/XDPoSChain that referenced this pull request May 13, 2024
@holiman @MariusVanDerWijden @gzliudan
core: transaction pool optimizations (ethereum#21328)
ff24635 
* core: added local tx pool test case

* core, crypto: various allocation savings regarding tx handling

* core/txlist, txpool: save a reheap operation, avoid some bigint allocs

Co-authored-by: Marius van der Wijden <[email protected]>
@gzliudan gzliudan mentioned this pull request May 17, 2024
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ucwong ucwong ucwong left review comments

@MariusVanDerWijden MariusVanDerWijden MariusVanDerWijden approved these changes

@karalabe karalabe Awaiting requested review from karalabe

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442 rjl493456442 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.9.17
Development

Successfully merging this pull request may close these issues.
3 participants
@holiman @MariusVanDerWijden @ucwong