-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: switch back to protobufjs #310
Conversation
d2d71ad
to
52f833d
Compare
@tuyennhv have you tried with new versions of protons?https://github.com/ipfs/protons/releases/tag/protons-v4.0.1 should have performance optimizations |
src/message/index.ts
Outdated
@@ -1 +0,0 @@ | |||
export * from './rpc.js' |
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.
We still need this file for the subpath export (import ... from '@chainsafe/libp2p-gossipsub/message'
)
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.
yeah this is exactly what I got when I build lodestar 👍
Codecov Report
@@ Coverage Diff @@
## master #310 +/- ##
==========================================
- Coverage 81.53% 79.83% -1.71%
==========================================
Files 43 44 +1
Lines 9501 11044 +1543
Branches 918 1014 +96
==========================================
+ Hits 7747 8817 +1070
- Misses 1754 2227 +473
Continue to review full report at Codecov.
|
@mpetrunic I've tried it, below is the result:
=> there is no improvement made with new protons and it's still 20x slower than protobufjs |
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!
Can you please share the benchmarks you're using to make these measurements? |
@achingbrain the benchmark is in #318 |
Motivation
Description
rpc.js
torpc.cjs
, newrpc.js
to reexportRPC
fromrpc.js