feat: use snappy-wasm #6483
Open
feat: use snappy-wasm #6483
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Benchmarks show `snappy` is faster at compressing and uncompressing larger payloads than `snappyjs`.
Performance Report✔️ no performance regression detected Full benchmark results
|
twoeths
reviewed
Feb 27, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #6483 +/- ##
============================================
+ Coverage 49.10% 49.23% +0.12%
============================================
Files 577 578 +1
Lines 37327 37434 +107
Branches 2136 2171 +35
============================================
+ Hits 18331 18429 +98
- Misses 18957 18965 +8
- Partials 39 40 +1 |
matthewkeil
reviewed
Feb 29, 2024
There was a problem hiding this comment.
The code LGTM! 🚀
Just the tiny question on the perf output but not something I feel strongly about. Truth be told we wont be looking at them much once this is merged. Neurosis is strong with this one... 😆 ... sigh....
Grafana is not wanting to load. Will click the approve once I can post some metrics here
|
Metrics for "till recieved" beacon block which should correlate to reduction in snappy decompression. Seeing slightly better speed on mainnet but similar results on the large validating nodes
|
twoeths
reviewed
Mar 4, 2024
| import {itBench} from "@dapplion/benchmark"; | ||
|
|
||
| describe("network / gossip / snappy", () => { | ||
| const msgLens = [100, 200, 300, 400, 500, 1000, 10000, 100000]; |
There was a problem hiding this comment.
I used to see blocks with 2MB, need to add 2MB, 5MB and 10MB (max size of gossip message) message length
|
the optimization is in |
|
looks like the metrics may not be panning out for this branch. It doesnt seem like there is a notable performance gain and the memory consumption is way up. Also seems a bit less stable with RSS bouncing around a lot on longer timeframes.
|
|
Just noting based on standup today that we will need to deploy via Docker for testing this one. |
|
A new contender has emerged (two years ago) Refreshed this branch and the benchmark results, check the description Also redeployed to feat3 |
|
Uploading a few juicy looking data points
|
wemeetagain
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Benchmarks show
snappy-wasmis faster at compressing and uncompressing thansnappyjs.Description
snappy-wasmfor compressing / uncompressing gossip payloadssnappyvssnappyjsvssnappy-wasmbenchmarksTODO
Closes #4170