-
-
Notifications
You must be signed in to change notification settings - Fork 435
fix: deserialize DataColumnSidecar without cloning Uint8Array #8626
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
Conversation
| seenTimestampSec, | ||
| }: GossipHandlerParamGeneric<GossipType.data_column_sidecar>) => { | ||
| const {serializedData} = gossipData; | ||
| const dataColumnSidecar = sszDeserialize(topic, serializedData); |
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.
if this turns out to be more efficient we need to think about upstreaming this to ssz, for example deserialize(true) for sharing Uint8Array and by default deserialize(false) to always clone it (which is what it currently is)
Performance Report✔️ no performance regression detected Full benchmark results
|
**Motivation** - ChainSafe/lodestar#8623 - ChainSafe/lodestar#8626 **Description** - Add `reuseBytes` option which causes `deserialize` to use [`Uint8Array#subarray`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/subarray) instead of [`Uint8Array#slice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice). This will cause the deserialized object to share the backing ArrayBuffer of the serialized payload. This is helpful, in some cases, to avoid additional allocations, however special care must be given to avoid corrupting memory using this option. The default behavior of `deserialize` is unchanged.
|
closing in favor of #8628 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #8626 +/- ##
============================================
- Coverage 51.96% 51.96% -0.01%
============================================
Files 848 848
Lines 65959 65955 -4
Branches 4814 4813 -1
============================================
- Hits 34275 34272 -3
+ Misses 31615 31614 -1
Partials 69 69 🚀 New features to boost your workflow:
|
Motivation
gcDescription
deserializeDataColumnSidecarUnsafe()to create a view of backed Uint8Array (Uint8Array.subarray()) instead of cloning it (Uint8Array.slice())Closes #8623
Testing
gc, which will lead to lower epoch transition (confirmed that too)DataColumnSidecaris tricky, will review with team later. Need to also pull a profile to make sure