Skip to content

Conversation

@unchuckable
Copy link
Contributor

This PR addresses AVRO-3048 (https://issues.apache.org/jira/projects/AVRO/issues/AVRO-3048), improving newBuilder() performance for newly generated SpecificRecords by reusing the already existing SpecificData instance (MODEL$) in the specific records themselves.

No new unit tests should be required, for the current changes are already covered by existing unit tests. Reference files for code generation have been updated to reflect the minor changes to the record and errror templates.

Further measures might be required to alleviate the performance regression of AVRO-3048 for existing code that cannot be re-generated easily.

If this PR is found feasible, I would suggest cherry-picking it for 1.9 and 1.10 branches, too, to allow for existing applications to benefit from the changes.

@github-actions github-actions bot added the Java Pull Requests for Java binding label May 1, 2021
@RyanSkraba RyanSkraba self-requested a review May 3, 2021 09:58
Copy link
Contributor

@RyanSkraba RyanSkraba left a comment

Choose a reason for hiding this comment

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

This LGTM! I think fixing the case for new generated records is the most important.

@RyanSkraba
Copy link
Contributor

Even if this looks fine for addressing the original issue specifically raised in the JIRA, I asked another question there based on one of your comments. Can you take a look before we merge this?

A quick question though, I can't see how your PR reduces any calls to newInstance(...) (which is also an expensive call). Am I mistaken?

@unchuckable
Copy link
Contributor Author

Even if this looks fine for addressing the original issue specifically raised in the JIRA, I asked another question there based on one of your comments. Can you take a look before we merge this?

A quick question though, I can't see how your PR reduces any calls to newInstance(...) (which is also an expensive call). Am I mistaken?

Good point, addressed this on JIRA.

@RyanSkraba RyanSkraba merged commit 1034773 into apache:master May 4, 2021
@unchuckable unchuckable deleted the AVRO-3048 branch May 4, 2021 14:51
unchuckable added a commit to unchuckable/avro that referenced this pull request Sep 13, 2021
* AVRO-3048: Use SpecificRecord's MODEL$ in builder creation

* AVRO-3048: fix formatting of code templates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants