Reduce checkpoint file size by using fewer bytes to encode length of encoded payload value #2165
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
In v0, length of encoded payload value is encoded in 8 bytes. In v1, length of encoded payload value is encoded in 4 bytes. Payload decoding handles both v0 and v1. Payload is encoded according to version received as parameter. This allows gradual migration of other encoding formats using payload, such as TrieUpdate and TrieProof.
fxamacker
requested review from
ramtinms,
m4ksio and
AlexHentschel
as code owners
Bump version of checkpoint file to v5. Support reading v4 for compatibility. Add tests and test data to read v4.
AlexHentschel
approved these changes
Mar 23, 2022
Update checkpoint file format version from v4 to v5
tarakby
approved these changes
Mar 23, 2022
| encMaxDepthSizeV4 = 2 | ||
| encRegCountSizeV4 = 8 | ||
| encHashSizeV4 = hash.HashLen | ||
| encPathSizeV4 = ledger.PathLen |
There was a problem hiding this comment.
👌🏼 for using the constants
fxamacker
deleted the
fxamacker/optimize-encoded-payload-value-length-in-checkpoint
branch
m4ksio
reviewed
Mar 23, 2022
| if err != nil { | ||
| log.Fatal().Err(err).Msg("invalid root checkpoint") | ||
| } | ||
| } else { |
There was a problem hiding this comment.
I think I left this comment on other PR - I would check if version is below one we explicitly support
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.
Description
Encode length of encoded payload value using 4 bytes (instead of 8 bytes) because the entire payload length is encoded to only 4 bytes.
This encoding is used in checkpoint, TrieUpdate (WAL), and TrieProof. Since this change affects code outside checkpoint, it was moved from PR #1944 to this PR.
Thanks @ramtinms and @tarakby for confirming we should reduce this length.
Closes #2070
Updates #1884
Updates https://github.com/dapperlabs/flow-go/issues/6114
Changes include:
Add payload encoding v1 to encode length of encoded payload value to 4 bytes. In v0, length of encoded payload value is encoded in 8 bytes.
Support decoding both v0 and v1.
Support version parameter for payload encoding. This allows gradual migration of other encoding formats using payload, such as TrieUpdate and TrieProof.
Caveats
This PR optimizes checkpoint file size which should reduce memory use. This isn't expected to have a big impact on speed. Other possible optimizations are outside the scope of this PR.