-
-
Notifications
You must be signed in to change notification settings - Fork 318
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: selectively use nodejs crypto for noise #5900
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
You should measure both cpu time and allocations, @tuyennhv 's version recycles the input buffers to reduce allocations |
4628944
to
87289e6
Compare
Before:
After:
|
roughly doubled the bytes that are gc'd. Eg: from ~800B per scavenge to ~1600B per scavenge |
87289e6
to
72d2b45
Compare
Deserves more testing. Since that blip, everything looks good. |
I deployed this branch to beta-mainnet node for 4 days the also I also compare to another test mainnet node, which have more gossipsub bandwidth (11.5k rpc/s vs 8k rpc/s in beta-mainnet node) and the time for |
NodeJS crypto tends to have better performance with longer message while lodestar mostly have messages with <512 bytes, I think we need to add benchmark for it @wemeetagain |
0709c3c
to
256f9af
Compare
Latest profile shows 7.26% of cpu time which is expected, gc is a little bit higher at 5.8% beta_mainnet_nodejs_crypto_Sep_15.cpuprofile.zip the interesting part is event loop lag way better than stable mainnet node, not sure if it only happens to beta mainnet node gossipsub traffic (or mesh peers) is only a little bit less than stable mainnet node |
🎉 This PR is included in v1.12.0 🎉 |
Motivation
Description