Skip to content

Adds support for FlexInt and FlexUInt in WriteBuffer#609

Merged
popematt merged 2 commits intoamazon-ion:masterfrom
popematt:flexint
Oct 16, 2023
Merged

Adds support for FlexInt and FlexUInt in WriteBuffer#609
popematt merged 2 commits intoamazon-ion:masterfrom
popematt:flexint

Conversation

@popematt
Copy link
Contributor

Issue #, if available:

None

Description of changes:

Adds support to the WriteBuffer for writing FlexInt and FlexUInt.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/com/amazon/ion/impl/bin/WriteBuffer.java 95.30% <100.00%> (+0.34%) ⬆️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@zslayton zslayton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

" 36028797018963968, 00000000 00000001 00000000 00000000 00000000 00000000 00000000 00000000 00000001",
" 4611686018427387903, 00000000 11111111 11111111 11111111 11111111 11111111 11111111 11111111 01111111",
" 4611686018427387904, 00000000 00000010 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000001",
" 9223372036854775807, 00000000 11111110 11111111 11111111 11111111 11111111 11111111 11111111 11111111 00000001",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this (and its negative counterpart below) are the limits for long. If that's the case, would you mind adding a comment stating that? It's a good test, and I assume many don't have the limits memorized.

" 562949953421311, 11000000 11111111 11111111 11111111 11111111 11111111 11111111",
" 562949953421312, 10000000 00000000 00000000 00000000 00000000 00000000 00000000 00000010",
"72057594037927935, 10000000 11111111 11111111 11111111 11111111 11111111 11111111 11111111",
"72057594037927936, 00000000 00000001 00000000 00000000 00000000 00000000 00000000 00000000 00000010",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the long positive limit?

@popematt popematt merged commit 3802e57 into amazon-ion:master Oct 16, 2023
@popematt popematt deleted the flexint branch October 16, 2023 18:30
Comment on lines +1315 to +1328
if (numBytes == 1) {
writeByte((byte) (0x01 | (byte)(value << 1)));
} else if (numBytes == 2) {
writeByte((byte) (0x02 | (byte)(value << 2)));
writeByte((byte) (value >> 6));
} else if (numBytes == 3) {
writeByte((byte) (0x04 | (byte)(value << 3)));
writeByte((byte) (value >> 5));
writeByte((byte) (value >> 13));
} else if (numBytes == 4) {
writeByte((byte) (0x08 | (byte)(value << 4)));
writeByte((byte) (value >> 4));
writeByte((byte) (value >> 12));
writeByte((byte) (value >> 20));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an optimization, we could go into the fallback loop branch if remaining() < numBytes, then in the short branches we can write directly into the buffer rather than repetitively calling writeByte, which requires some redundant logic. See e.g. writeUInt64 vs writeUInt64Slow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants