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

Add copyTo() method to bytes #827

Closed
keith-turner opened this Issue Apr 26, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@keith-turner
Contributor

keith-turner commented Apr 26, 2017

For data serialization use cases, a user may want to copy multiple Bytes objects to a byte array. The current options for that are to call Bytes.toArray() or use the Bytes.byteAt() method. If Bytes had a copyTo(byte[] dest, int offset) method then it could support this use case more efficiently. The following is an example.

Bytes field1 = Bytes.of("foo");
Bytes field2 = Bytes.of("bar");

byte[] dest = new byte[field1.length() + field2.lenght() + 1];

field1.copyTo(dest, 0);
dest[field1.lenght()] = ':';
field2.copyTo(dest, field1.lenght()+1);
@ctubbsii

This comment has been minimized.

Show comment
Hide comment
@ctubbsii

ctubbsii Apr 26, 2017

Member

Looking at this example, I'm wondering if the javadoc for any such method should make it a point to remind the user that the offset and length correspond to the UTF-8 encoded byte array form of any String/CharSequence that they may have used to construct the Bytes with, and may differ from the character offsets in the original string.

Member

ctubbsii commented Apr 26, 2017

Looking at this example, I'm wondering if the javadoc for any such method should make it a point to remind the user that the offset and length correspond to the UTF-8 encoded byte array form of any String/CharSequence that they may have used to construct the Bytes with, and may differ from the character offsets in the original string.

@keith-turner

This comment has been minimized.

Show comment
Hide comment
@keith-turner

keith-turner May 8, 2017

Contributor

For the case where a subsequence of a Bytes object needs to be copied, I was thinking the subsequence() method could be used. For example the following could copy the 1st 255 bytes of b1 to dest starting at offset 1024.

byte[] dest = ...

Bytes b1;

b1.subsequence(0,255).copyTo(dest, 1024);

However, calling subsequence will allocate an intermediate Bytes object. To avoid this, we could have a method like copyTo(int sourceOffset, byte[] dest, int destOffset, int len) instead of copyTo(byte[] dest, int offset).

Contributor

keith-turner commented May 8, 2017

For the case where a subsequence of a Bytes object needs to be copied, I was thinking the subsequence() method could be used. For example the following could copy the 1st 255 bytes of b1 to dest starting at offset 1024.

byte[] dest = ...

Bytes b1;

b1.subsequence(0,255).copyTo(dest, 1024);

However, calling subsequence will allocate an intermediate Bytes object. To avoid this, we could have a method like copyTo(int sourceOffset, byte[] dest, int destOffset, int len) instead of copyTo(byte[] dest, int offset).

@mjwall

This comment has been minimized.

Show comment
Hide comment
@mjwall

mjwall May 9, 2017

Member

I can take a swing at this one. Is there a way for us to assign issues in GH?

Member

mjwall commented May 9, 2017

I can take a swing at this one. Is there a way for us to assign issues in GH?

@keith-turner

This comment has been minimized.

Show comment
Hide comment
@keith-turner

keith-turner May 10, 2017

Contributor

Not yet, but hopefully soon INFRA-14121

Contributor

keith-turner commented May 10, 2017

Not yet, but hopefully soon INFRA-14121

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 11, 2017

@keith-turner keith-turner added this to the 1.1.0 milestone May 17, 2017

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

refs #827 - implement a copyTo
that uses a new Bytes.arraycopy method.  That last test in BytesTests may a
little much, but I had to prove to myself how System.arraycopy was working.

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

mjwall added a commit to mjwall/incubator-fluo that referenced this issue May 18, 2017

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