apache
AVRO-4060: Use JDK to Hash Byte Array in UTF8
What is the purpose of the change
- This pull request improves file read performance by using the JDK hashcode implementation, fixing AVRO-4060.*
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Documentation
- Does this pull request introduce a new feature? no
I feel there should be a test to verify that the result of Utf8.hashCode does not depend on whether the array has unused bytes in it. TestUtf8.hashCodeReused seems to largely cover that by hardcoding specific hash codes but it does not compute hash codes for equal sequences of bytes in two ways. I'm thinking about test code like
Utf8 u1 = new Utf8("abcdefghi"); // length=9, bytes.length=9
u1.setByteLength(8); // length=8, bytes.length=9
Utf8 u2 = new Utf8("abcdefgh"); // length=8, bytes.length=8
assertEquals(u1.hashCode(), u2.getHashCode());
or with a hardcoded hash code in the test.
Hello,
Thank you for the feedback. I agree a unit test is appropriate here.
Ideally, the hashcode implementation should return the same value for the same contents regardless on the size of the array. However, that is not currently the case because the two implementations are different. I believe AVRO-4061 will fix the discrepancy between the two implementations and then this unit test becomes possible.
I also need to look at the setLength method. It's very confusing to me. If the buffer is made smaller, then the underlying string is truncated, if the buffer is made larger, than the underlying string is padded with zero (it's not just that the buffer is expanded). I don't really understand the use case of this method just yet. I feel like it shouldn't bother copying anything into the newly sized array since the end result is somewhat confusing.
I have a unit test that confirms the behavior, but it does require AVRO-4061 to be merged first.