-
Notifications
You must be signed in to change notification settings - Fork 530
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
Drop support for v0 and v1 blocks #919
Drop support for v0 and v1 blocks #919
Conversation
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
This is ready to go, but leaving draft status up b/c I want to merge #806 first. There will be some small conflicts. |
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
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.
LGTM, just a minor non-blocking nit
if cap(pagesBuffer) < len(compressedPages) { | ||
// extend pagesBuffer | ||
diff := len(compressedPages) - cap(pagesBuffer) | ||
pagesBuffer = append(pagesBuffer[:cap(pagesBuffer)], make([][]byte, diff)...) |
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.
append()
will copy over all the items from old to new array right? and the resultant pagesBuffer array might have a cap larger than len(compressedPages)
, worth just creating a new array with cap(pagesBuffer) = len(compressedPages)
? The reuse is primarily useful when cap(pagesBuffer) > len(compressedPages)
I guess
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.
and the resultant pagesBuffer array might have a cap larger than len(compressedPages)
I believe after this line len(pagesBuffer) = cap(pagesBuffer) = len(compressedPages)
worth just creating a new array with cap(pagesBuffer) = len(compressedPages)
by using whatever currently exists in pagesBuffer we reduce byte slice allocations. after this line the first N entries in pagesBuffer already have allocated byte slices that can be reused in ReadAllWIthBuffer
. if we recreated the slice we would lose that.
What this PR does:
Drops support for v0 and v1 blocks and consolidates all code into v2.
TODO
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]