avro icon indicating copy to clipboard operation
avro copied to clipboard

AVRO-3527: Generated equals() and hashCode() for SpecificRecords

Open steven-aerts opened this issue 3 years ago • 6 comments

Update the compiler to generate the implementation of the .equals() and `.hashCode() function, instead of relying on the implementation of GenericData. This improves the performance of those functions significantly.

The generated implementations are factor 10 to 20 faster for .equals() and a factor 5 to 10 for .hashCode().

The implementation generates the same hashCode as the genericData, which is validated by existing tests

Result of Perf test before the change:

Benchmark              Mode  Cnt          Score             Error  Units
SpecficTest.equals    thrpt    3   12598610.194 +/-  11160265.279  ops/s
SpecficTest.hashCode  thrpt    3   24729446.862 +/-  29051332.794  ops/s

Results using generated functions:

Benchmark              Mode  Cnt          Score             Error  Units
SpecficTest.equals    thrpt    3  211314296.950 +/- 104154793.126  ops/s
SpecficTest.hashCode  thrpt    3  180349506.632 +/- 143639246.771  ops/s

Jira

  • [x] My PR addresses the following: AVRO-3527 Generated equals() and hashCode() for SpecificRecords

Tests

  • [x] My PR adds the following unit tests:
  • TestUtf8#testHashCodeSameAsString()
  • TestGeneratedCode#ignoredFields()
  • JMH test for SpecificRecords equals() and hashCode()

Commits

  • [x] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • [x] In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

steven-aerts avatar Jun 03 '22 05:06 steven-aerts

Thanks for the PR, this looks really interesting! I set the target for 1.12.0 because it's a change in how classes are generated... but I'm curious whether that should really be the case. I really don't think there's any reason this could be a major change! What do you think?

RyanSkraba avatar Jun 07 '22 16:06 RyanSkraba

@RyanSkraba there is one small corner case, which let me doubt.

When using CharSequence as stringType, the equals method uses a newly introduced Utf8#compareSequences. As we could not use java.lang.CharSequence#compare which is only available since java 11.

If we choose to introduce this change as a minor version. It would mean that for the Charsequence stringType, the code generated by an 1.11.1 compiler is incompatible with a 1.11.0 avro library. Everything else should work in that constellation.

Do you see that as a reason to introduce it in a major version only?

steven-aerts avatar Jun 08 '22 06:06 steven-aerts

Do you see that as a reason to introduce it in a major version only?

Thanks! I think it's a good idea to bump it to the next major version, but we usually do one later in the year.

I used to think that Avro required that the code-generator version must be in sync with the avro library version, because that's the way my company always did it, but I've learned since that it's not always the case.

RyanSkraba avatar Jun 08 '22 12:06 RyanSkraba

I used to think that Avro required that the code-generator version must be in sync with the avro library version, because that's the way my company always did it, but I've learned since that it's not always the case.

This is the case for ANTLR, and is a common practice because some generated code is so tightly coupled to the library, that using the same version is a requirement to ensure compatibility. This is not the case for Avro.

opwvhk avatar Jun 09 '22 10:06 opwvhk

@RyanSkraba is there still something I can do to help to get this change submitted?

steven-aerts avatar Jul 25 '22 09:07 steven-aerts

Hey, thanks so much for your patience! The RC1 for 1.11.1 is out for testing, so hopefully we should be getting around to PRs targetted for 1.12.0 in August.

RyanSkraba avatar Jul 27 '22 08:07 RyanSkraba