Skip to content
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
merged 5 commits into from
Sep 12, 2023

Conversation

fxamacker
Copy link
Member

@fxamacker fxamacker commented Sep 5, 2023

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.).


  • Targeted PR against main branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

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.
@fxamacker fxamacker added the enhancement New feature or request label Sep 5, 2023
@fxamacker fxamacker self-assigned this Sep 5, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2023

Codecov Report

Patch coverage: 62.23% and project coverage change: -0.40% ⚠️

Comparison is base (74e19aa) 65.24% compared to head (a5c7323) 64.84%.

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     
Files Changed Coverage Δ
basicarray.go 49.36% <14.28%> (-0.42%) ⬇️
slab.go 66.66% <40.00%> (-33.34%) ⬇️
storable_slab.go 27.53% <50.00%> (-4.35%) ⬇️
encode.go 76.47% <57.14%> (-2.90%) ⬇️
array.go 69.91% <60.25%> (-0.63%) ⬇️
map.go 67.05% <64.10%> (-0.31%) ⬇️
flag.go 74.21% <72.61%> (-2.06%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.
…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
@fxamacker fxamacker merged commit 1e6ec55 into main Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants