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

libp2p-mplex decode performance issue #3467

Closed
twoeths opened this issue Nov 30, 2021 · 2 comments · Fixed by #3489
Closed

libp2p-mplex decode performance issue #3467

twoeths opened this issue Nov 30, 2021 · 2 comments · Fixed by #3489
Assignees
@twoeths
Labels
scope-performance Performance issue and ideas to improve performance.

Comments

@twoeths
Copy link
Contributor

twoeths commented Nov 30, 2021

Describe the bug

  • With subscribeToAllSubnets() flag, it takes 5% of cpu time to decode libp2p-mplex chunks

Screen Shot 2021-11-30 at 14 52 57

  • 90% of the time was used to throw RangeError

Screen Shot 2021-11-30 at 14 54 14

1130_lodestar_subscribeAllSubnets_local.cpuprofile.zip
@dapplion
Copy link
Contributor

Oh interesting, could you check the metrics to see if there were many invalid messages during the time that the CPU profile was taken? How could it be that we get sent so many invalid messages? Or the consumer code of this library is doing something strange and passing bad data to the decoder?

@dapplion dapplion added the scope-performance Performance issue and ideas to improve performance. label Nov 30, 2021
@twoeths
Copy link
Contributor Author

twoeths commented Nov 30, 2021

@dapplion it's by design, after decoding a chunk's header successfully, the buffer is consumed the whole length, and it has 0 length, and varint.decode() will just throw RangeError in that case. This happens 50% of the time libp2p-mplex uses varint.decode() in that function

Screen Shot 2021-11-30 at 15 04 52

this could be a simple fix, will create a PR for it

@twoeths twoeths mentioned this issue Nov 30, 2021
Merged
achingbrain pushed a commit to libp2p/js-libp2p-mplex that referenced this issue Dec 7, 2021
@twoeths
perf: do not call varint.decode() if buffer has 0 length (#125)
92f1727 
As noted in ChainSafe/lodestar#3467, 80%-90% of the `varint.decode()` function time is used to handle `varint.decode()` RangeError because the buffer length is 0
@twoeths twoeths mentioned this issue Dec 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@twoeths twoeths

Labels
scope-performance Performance issue and ideas to improve performance.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update to libp2p-mplex 0.10.5 ChainSafe/lodestar
2 participants
@twoeths @dapplion