-
Notifications
You must be signed in to change notification settings - Fork 12
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
Refactor encoding version and flag to add more flags #338
Merged
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
Currently, version number uses 1 byte and flags uses 1 byte. We don't need 255 versions but we need more flags in order to support atree inlining. For now, reserve the first 4 bits for version, but also reserve the 5th bit separately so in the future, we can decide if we want to use 5th bit to add 16 more versions or a new flag. So the first byte has (beginning with top bit): 1-4: version (0 to 15) 5: reserved to provide 16 more versions or another flag 6: reserved (use this next if needed) 7: slab has next slab ID (only relevant for data slab) 8: slab has inlined slabs (for atree inlining) This change is backward compatible.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
- Coverage 65.24% 64.84% -0.40%
==========================================
Files 14 14
Lines 8761 8787 +26
==========================================
- Hits 5716 5698 -18
- Misses 2327 2356 +29
- Partials 718 733 +15
☔ View full report in Codecov by Sentry. |
Currently, we omit empty next slab ID in encoded root data slabs because next slab ID is always empty in root data slabs. However, next slab ID is also empty for non-root data slabs if the non-root data slab is the last data slab. This commit sets hasNextSlabID flag during encoding and only encodes non-empty next slab ID for array data slab. This change saves 16 bytes for the last non-root data slabs. Also, we don't special case the omission of next slab ID in root slabs. NOTE: omission of empty next slab ID doesn't affect slab size computation which is used for slab operations, such as splitting and merging. This commit is an optimization during slab encoding.
Currently, we omit empty next slab ID in encoded root data slabs because next slab ID is always empty in root data slabs. However, next slab ID is also empty for non-root data slabs if the non-root data slab is the last data slab. This commit sets hasNextSlabID flag during encoding and only encodes non-empty next slab ID for map data slab. This change saves 16 bytes for the last non-root data slabs. Also, we don't special case the omission of next slab ID in root slabs. NOTE: omission of empty next slab ID doesn't affect slab size computation which is used for slab operations, such as splitting and merging. This commit is an optimization during slab encoding.
ramtinms
approved these changes
Sep 6, 2023
…id-in-encoded-map-data Omit empty next slab ID in encoded map data slab
…id-in-encoded-array-data Omit empty next slab ID in encoded array data slab
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.
Updates #292 onflow/flow-go#1744
Changes:
Currently, version number uses 1 byte and flags uses 1 byte.
We don't need 255 versions but we need more flags now in order to support atree inlining.
For now, reserve the first 4 bits for version (0-15), but also reserve the 5th bit separately so in the future, we can decide if we want to use 5th bit to have more versions (0-31) or a new flag.
So the first byte has (beginning with top bit):
1-4: version (0 to 15) and version 1 only uses 4th bit.
5: reserved to provide 16 more versions or another flag
6: reserved (use this for next new flag if needed)
7: slab has next slab ID (only relevant for data slab)
8: slab has inlined slabs (for atree inlining)
Atree inlining bumps version from 0 to 1. The top bit won't be needed until version 7, so it can be used to change this layout in the future if needed.
This PR is backward compatible and simpler than alternatives (e.g. doesn't reverse top bits, etc.).
main
branchFiles changed
in the Github PR explorer