SSZ v2 #223
Merged
SSZ v2 #223
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
dapplion
requested review from
ColinSchwarz,
RyRy79261,
twoeths and
wemeetagain
as code owners
Performance Report✔️ no performance regression detected 🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
Merged
twoeths
reviewed
Jan 18, 2022
twoeths
reviewed
Jan 18, 2022
twoeths
reviewed
Jan 18, 2022
twoeths
reviewed
Jan 18, 2022
twoeths
reviewed
Jan 18, 2022
wemeetagain
requested changes
Jan 20, 2022
Comment on lines
+403
to
+512
| return startIndex === 0 ? [rootNode.left] : [rootNode.right]; | ||
| } else { | ||
| return [rootNode.left, rootNode.right]; |
There was a problem hiding this comment.
These are a little optimistic, no? should have more validation of count and startIndex
some test cases that should error:
getNodesAtDepth(node, 1, 5, 1) - startIndex > maximum index at depth
getNodesAtDepth(node, 1, 0, 4) - count > maximum count at depth
getNodesAtDepth(node, 1, 1, 2) - startIndex + count > maximum index at depth
previously caught with this condition:
if (BigInt(1) << BigInt(depth) < startIndex + count) {
throw new Error(ERR_COUNT_GT_DEPTH);There was a problem hiding this comment.
Yeah this function is optimistic, as well as setNodesAtDepth where you must pass data sorted, and perfectly arranged, or the result is wrong in hard to notice ways. This methods are meant to achieve the max performance possible. getNodesAtDepth is a pretty hot path when deserializing a full state. Do you really think it should protect against the test cases you listed above?
wemeetagain
requested changes
Jan 26, 2022
wemeetagain
reviewed
Jan 26, 2022
g11tech
reviewed
Feb 2, 2022
dapplion
force-pushed
the
dapplion/v2
branch
2 times, most recently
from
94f8749
to
1bf9b1c
Compare
Replace lodestarTypes Update tests Add Union and None types Update spec tests Remove old src and lodestar dependencies Improve JSON parsing for tests Load YAML with lodestar utils schema Commit ArrayComposite before getAll Re-write toJson fromJson test Fix uint json conversions Print tree in valid test if requested Fix hashTreeRoot for ByteList Add RENDER_ROOTS option Fix typos Use numerical sort in array commit Fix value_serializedSizeArrayComposite Pass ssz_generic tests Allow to select tests to run in ssz_static Fix container bytes offset Rename fixedLen to fixedSize Sort deserailization methods Print json stringified in test Review logic Clean up tests Define JSON casing at constructor time only Add casing maps for merge types Update casing in unit tests Re-organize utils Fix merge casing FIx offset calculation Pass all spec test pre-merge Bump merge test Extend timeout for mainnet tests Pass all unit tests Return defaultValue in simpleserialize random Remove @chainsafe/lodestar-spec-test-util dependency Copy yaml schema from lodestar-utils Fix benchmark type issues Skip createProof benchmark Skip old benchmark without runner Remove postinstall script Re-add UintBigint optimization Fix set_exitEpoch_and_hashTreeRoot benchmark Add List of Number benchmark Use DataViews for faster deserialization Use DataViews for tree serialization too Update workflows build after install Update packedNode tests Review Tree API Validate length in ByteArrays FIx benchmarks in persistent-merkle-tree Simplify LeafNode constructor Refactor subtreeFillToContents Run struct <-> tree_backed benchmarks Update test types Fix Uint64 DataView benchmarks Add benchmarks for full state serialization Use consistent initialization in DataView Optimize toView for ByteArray Add clone method for safer ContainerTreeViewDU Add documentation to all public methods Update SSZ README Simplify testTypes Update persistent-merkle-tree README Add unit test push x5 Fix heigh typo Add note Supports index up to Number.MAX_SAFE_INTEGER. Address PR comments Add more benchmarks (#227) Remove merkleizeSingleBuff Add more documentation Add typeName to all containers Add eth2 JSON casing Rename merge to bellatrix Fix missing renames of merge to bellatrix Update ssz readme Export byteArrayEquals Add more BitArray functionality Various fixes Test type UX of allForks types Abstract logic into BitArray and ByteArray Add .equals functionality Don't run allForks code Rename receiptRoot Update ssz docstrings Multiple fixes Remove non-existent variable Add proof api Fix int conversion in node.getUintBigint Add Proofs functionality Run Proof tests on ssz_static minimal Add length node for proofs of lists Require rootNode only when necessary in Array proofs Fix rebase issues Move Node navigation to functions Add treeZeroAfterIndex fn Add sliceTo method in CompositeList ViewDU Use Uint64NumInf only where necessary Add Eth1Block type Use defaultView and defaultViewDU methods Support BranchNodeStruct in proofs Run proof tests Post process proof nodes Fix proof generation for all types Fix ContainerNodeStructType proof unit test Stop tracking complimentary benchmarks Fix typo in Union maxSize Add test for maxSize and minSize for all data structures Export helper functions Fix length param in Vector Prevent setting values in ViewDU beyond length Add more test cases Improve ListBasicTreeViewDU.push logic Fix Composite ViewDU commit nodes logic Fix cache logic in ViewDU Use type guards and simply set composite Fix allForks test Fix recursive call in BitArray Ensure data consistency in ArrayCompositeViewDU Better error in BitArray.set FIx typo in populateAllNodes Add more BitArray unit tests TreeViewDU.commit should not return node Better error messages in fromHexString Auto-commit on ViewDU Fix perf types Fix proof generation for Validator type Benchmark more eth2 ssz objects Track raw cost of hashing vc list Add more coverage WIP More coverage WIP Prevent pushing over length More coverage Ensure consistent mutability behaviour Ensure correct mutability in ContainerNodeStruct Move isViewMutable to CompositeType Prevent keeping references for immutable views Fix ContainerNodeStruct valid tests Fix types in Uint constructor Increase bitArray coverage Guard against new BitList checks Fix readVariableOffsetsArrayComposite
dapplion
force-pushed
the
dapplion/v2
branch
from
1bf9b1c
to
59598e4
Compare
|
I've squashed all commits into one for better visibility of this PR's comments and reviews. All commits will be squashed on merge so it's not important to preserve git history of this branch at this point |
8 tasks
|
Is this still getting merged in @dapplion as discussed on the Mar 18 planning call? |
Yes @wemeetagain should help merge and release this |
wemeetagain
approved these changes
Mar 23, 2022
wemeetagain
pushed a commit
that referenced
this pull request
Mar 24, 2022
BREAKING CHANGE: complete refactor, see packages/ssz/README.md for details Initial squashed v2 commit Replace lodestarTypes Update tests Add Union and None types Update spec tests Remove old src and lodestar dependencies Improve JSON parsing for tests Load YAML with lodestar utils schema Commit ArrayComposite before getAll Re-write toJson fromJson test Fix uint json conversions Print tree in valid test if requested Fix hashTreeRoot for ByteList Add RENDER_ROOTS option Fix typos Use numerical sort in array commit Fix value_serializedSizeArrayComposite Pass ssz_generic tests Allow to select tests to run in ssz_static Fix container bytes offset Rename fixedLen to fixedSize Sort deserailization methods Print json stringified in test Review logic Clean up tests Define JSON casing at constructor time only Add casing maps for merge types Update casing in unit tests Re-organize utils Fix merge casing FIx offset calculation Pass all spec test pre-merge Bump merge test Extend timeout for mainnet tests Pass all unit tests Return defaultValue in simpleserialize random Remove @chainsafe/lodestar-spec-test-util dependency Copy yaml schema from lodestar-utils Fix benchmark type issues Skip createProof benchmark Skip old benchmark without runner Remove postinstall script Re-add UintBigint optimization Fix set_exitEpoch_and_hashTreeRoot benchmark Add List of Number benchmark Use DataViews for faster deserialization Use DataViews for tree serialization too Update workflows build after install Update packedNode tests Review Tree API Validate length in ByteArrays FIx benchmarks in persistent-merkle-tree Simplify LeafNode constructor Refactor subtreeFillToContents Run struct <-> tree_backed benchmarks Update test types Fix Uint64 DataView benchmarks Add benchmarks for full state serialization Use consistent initialization in DataView Optimize toView for ByteArray Add clone method for safer ContainerTreeViewDU Add documentation to all public methods Update SSZ README Simplify testTypes Update persistent-merkle-tree README Add unit test push x5 Fix heigh typo Add note Supports index up to Number.MAX_SAFE_INTEGER. Address PR comments Add more benchmarks (#227) Remove merkleizeSingleBuff Add more documentation Add typeName to all containers Add eth2 JSON casing Rename merge to bellatrix Fix missing renames of merge to bellatrix Update ssz readme Export byteArrayEquals Add more BitArray functionality Various fixes Test type UX of allForks types Abstract logic into BitArray and ByteArray Add .equals functionality Don't run allForks code Rename receiptRoot Update ssz docstrings Multiple fixes Remove non-existent variable Add proof api Fix int conversion in node.getUintBigint Add Proofs functionality Run Proof tests on ssz_static minimal Add length node for proofs of lists Require rootNode only when necessary in Array proofs Fix rebase issues Move Node navigation to functions Add treeZeroAfterIndex fn Add sliceTo method in CompositeList ViewDU Use Uint64NumInf only where necessary Add Eth1Block type Use defaultView and defaultViewDU methods Support BranchNodeStruct in proofs Run proof tests Post process proof nodes Fix proof generation for all types Fix ContainerNodeStructType proof unit test Stop tracking complimentary benchmarks Fix typo in Union maxSize Add test for maxSize and minSize for all data structures Export helper functions Fix length param in Vector Prevent setting values in ViewDU beyond length Add more test cases Improve ListBasicTreeViewDU.push logic Fix Composite ViewDU commit nodes logic Fix cache logic in ViewDU Use type guards and simply set composite Fix allForks test Fix recursive call in BitArray Ensure data consistency in ArrayCompositeViewDU Better error in BitArray.set FIx typo in populateAllNodes Add more BitArray unit tests TreeViewDU.commit should not return node Better error messages in fromHexString Auto-commit on ViewDU Fix perf types Fix proof generation for Validator type Benchmark more eth2 ssz objects Track raw cost of hashing vc list Add more coverage WIP More coverage WIP Prevent pushing over length More coverage Ensure consistent mutability behaviour Ensure correct mutability in ContainerNodeStruct Move isViewMutable to CompositeType Prevent keeping references for immutable views Fix ContainerNodeStruct valid tests Fix types in Uint constructor Increase bitArray coverage Guard against new BitList checks Fix readVariableOffsetsArrayComposite Fix rebase issues WIP - hashTreeRoot perf Pre-allocate array if length is known Use Uint32Array in offsets array Expose faster ArrayLike vectors for view.getAll() Allow to customize Array typename Add more array perf tests Remove ArrayLikeWritable Increase coverage Modularize TreeView and TreeViewDU More coverage Remove rebindLeft rebindRight methods Add more coverage Organize tests Set name to Container tree view instances export CompositeTypeAny Refactor default* getters to methods
Merged
Closed
Closed
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
Our current library has multiple fundamental limitations that affect performance, memory and usability downstream. In Lodestar we have a added a lot of custom code to get around this limitations. This PR up-streams all the good ideas created downstream into a library that's performant by default without requiring extra infrastructure
Features
TreeViewDU
Stands for TreeView Deferred Update, and allows for very fast reads and writes without committing to the tree until latter. Then is optimized batched operations all nodes are recomputed and added to the tree with a fast routine
setNodesAtDepth.This mode is meant to be used in the entire beacon state transition function. This is a fundamental shift from the previous version where tree views and regular values had the same API. However, achieving this requires ES6 Proxy which have performance issues. Now the beacon state transition function will not compile if regular JS values are passed to it, it will only accept the TreeViewDU representation of the state.
DataView and Uint8Array for faster (de)serialization
Current version spends significant time converting bytes to decimal numbers. DataView is extremely fast at bytes to decimal conversion and back. This PR uses it for value conversions and to populate HashObjects much faster.
However,
Uint8Array.sliceis by far the fastest way of copying binary data. So this PR passes both a DataView and a Uint8Array to make decimal conversions and data copying as fast as possible.Typed types
The Typescript types of values, TreeView and TreeViewDU are derived from the type arguments. Previously the SSZ type and its Typescript type had no assurance that they were correct and represented the same data. With this guarantee we can now use novel type representations and iterate quickly without breaking any code downstream.
Value hash cache
Previously only TreeViews had the ability to cache hashTreeRoot results, so in Lodestar we deserialized Attestations as tree just for that. This version introduces the option
cachePermanentRootStructfor composite types which will cache only the root hashTreeRoot (not other intermediary hashes) in the JavaScript object itself. Now we can deserialize Attestations as value and get cached hashTreeRoot in a memory efficient and performant representation.BitArray
BitList and BitVectors are now represented in a new class
BitArraythat replaces the previousboolean[]representation. Thanks to some optimizations introduced in Lodestar to intersect Uint8Arrays fast it provides the same performance as aboolean[]but at a much lower memory cost. Now in Lodestar we don't need to do the optimization manually and just callPre-computed type data
Many of the types immutable characteristics like minSize, fixedLen, offset positions, etc are now computed in the constructor. This makes consuming this data instant speeding up serialization + de-serialization. Containers also pre-compute offset data and JSON keys data, which help speed up its methods.
TODO after merge