Skip to content

clist: speedup Next by removing defers#2511

Merged
ebuchman merged 1 commit intodevelopfrom
dev/speedup_reap
Sep 30, 2018
Merged

clist: speedup Next by removing defers#2511
ebuchman merged 1 commit intodevelopfrom
dev/speedup_reap

Conversation

@ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Sep 29, 2018 •
edited
Loading

This change doubles the speed of the mempool's reaping.
Before:

    BenchmarkReap-8         5000            365390 ns/op          122887 B/op

After:

    BenchmarkReap-8        10000            158274 ns/op          122882 B/op
  • Updated all relevant documentation in docs - n/a
  • Updated all code comments where relevant - n/a
  • Wrote tests
  • Updated CHANGELOG_PENDING.md - not sure this needs a changelog entry

@ValarDragon
clist: speedup Next by removing defers
7a01161 
This change doubles the speed of the mempool's reaping.
Before:

BenchmarkReap-8   	    5000	    365390 ns/op	  122887 B/op

After:

BenchmarkReap-8   	   10000	    158274 ns/op	  122882 B/op
@ValarDragon
Copy link
Contributor Author

ValarDragon commented Sep 29, 2018

The time breakdown of mempool reaping is now:

62.5% of the time is spent on mutex unlocking / locking in clist.Next
11% of the time is spent in allocating the outputted tx list
11% is spent on amino UvarintSize. (I've submitted a fix for its speed which will be included in the next amino release)
2% of the time is spent on CheckTx

@codecov-io
Copy link

codecov-io commented Sep 29, 2018

Codecov Report

Merging #2511 into develop will increase coverage by 0.27%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #2511      +/-   ##
===========================================
+ Coverage    61.26%   61.53%   +0.27%     
===========================================
  Files          202      197       -5     
  Lines        16710    16450     -260     
===========================================
- Hits         10237    10123     -114     
+ Misses        5607     5497     -110     
+ Partials       866      830      -36
Impacted Files Coverage Δ
libs/clist/clist.go 66.66% <100%> (+0.16%) ⬆️
lite/dbprovider.go 59.18% <0%> (-18.37%) ⬇️
lite/proxy/query.go 0% <0%> (-9.81%) ⬇️
lite/dynamic_verifier.go 56.25% <0%> (-5.94%) ⬇️
privval/socket.go 71.48% <0%> (-4.32%) ⬇️
p2p/peer.go 59% <0%> (-3.73%) ⬇️
cmd/tendermint/commands/lite.go 13.15% <0%> (-2.23%) ⬇️
crypto/merkle/simple_proof.go 65.11% <0%> (-2.2%) ⬇️
privval/priv_validator.go 69.44% <0%> (-1.58%) ⬇️
abci/example/kvstore/kvstore.go 80.64% <0%> (-0.31%) ⬇️
... and 12 more

melekes
melekes approved these changes Sep 29, 2018
View reviewed changes
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

🍰 🌮 🦀

@ebuchman
Copy link
Contributor

ebuchman commented Sep 30, 2018

62.5% of the time is spent on mutex unlocking / locking in clist.Next

yikes!

@ebuchman ebuchman merged commit 69c7aa7 into develop Sep 30, 2018
@ebuchman ebuchman deleted the dev/speedup_reap branch September 30, 2018 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xla xla Awaiting requested review from xla

2 more reviewers

@melekes melekes melekes approved these changes

@ebuchman ebuchman ebuchman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ValarDragon @codecov-io @ebuchman @melekes