AVRO-4060: Use JDK to Hash Byte Array in UTF8

[belugabehr]

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

[KalleOlaviNiemitalo]

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.

[belugabehr]

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.

[belugabehr]

I have a unit test that confirms the behavior, but it does require AVRO-4061 to be merged first.

[belugabehr]

Unit test will fail on this PR until AVRO-401 is addressed.

[belugabehr]

AVRO-4061 is now fixed.

Added unit test for coverage.

[belugabehr]

@KalleOlaviNiemitalo and @martin-g - Another pass on this one please?

[KalleOlaviNiemitalo]

Some nits on the javadoc but otherwise it looks OK. I'm not a professional Java programmer, though.

[KalleOlaviNiemitalo]

"hashcodehashCode" (twice)

"function can isbe vectorized in JDK 21+"

The "eq." and "le." abbreviations look confusing. Does "le." mean "less than or equal to"? But the equality case was already described.

[martin-g]

This function can is vectorized ... sounds incorrect

[martin-g]

How does Arrays.hashCode(bytes) behave when the length is smaller ?
Doesn't it fall back to serial execution internally for anything that is not vectorizable ?

[belugabehr]

It does fall back, but in my micro benchmarks, I found that it is better to implement this skip logic before bothering to jump into the method call itself especially since the length needs to be interrogated anyway.