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

BytesReference.Helper should never materialize a byte[] array. #5517

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 24, 2014

The current implementations of BytesReference.Helper.bytesEquals and
bytesHashCode materialize a byte[] when passed an instance that returns false
in hasArray(). They should instead do a byte-by-byte comparison/hashcode
computation without materializing the array.

The current implementations of BytesReference.Helper.bytesEquals and
bytesHashCode materialize a byte[] when passed an instance that returns `false`
in `hasArray()`. They should instead do a byte-by-byte comparison/hashcode
computation without materializing the array.

Close elastic#5517
}

// pkg-private for testing
static boolean slowBytesEquals(BytesReference a, BytesReference b) {
Copy link
Member

Choose a reason for hiding this comment

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

should we assert that they have the same length here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point!

@kimchy
Copy link
Member

kimchy commented Mar 24, 2014

LGTM except for my minor comment

@hhoffstaette
Copy link

While we're touching this can we please make sure the hashes are compatible with Arrays.hashCode()? Right now they are not since the initial hash starts at 0, not 1. PagedBytesReference and the initial bits in BigArrays already do the right thing.

@jpountz
Copy link
Contributor Author

jpountz commented Mar 25, 2014

I just added a new commit to change the initial hash code to 1 and add the assertion that @kimchy suggested. I'll merge soon.

@jpountz jpountz closed this in badedf6 Mar 25, 2014
jpountz added a commit that referenced this pull request Mar 25, 2014
The current implementations of BytesReference.Helper.bytesEquals and
bytesHashCode materialize a byte[] when passed an instance that returns `false`
in `hasArray()`. They should instead do a byte-by-byte comparison/hashcode
computation without materializing the array.

Close #5517
@jpountz jpountz deleted the fix/BytesReferenceHelper_byte_materialization branch March 25, 2014 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants