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

HHH-16809 + HHH-18976 Avoid usage of Array.newInstance + Add JavaType#newArray #9576

Closed
wants to merge 11 commits into from

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Jan 6, 2025

Alternative to #9569 that also solves HHH-16809 (though that part is still WIP, see WIP commit).

Some early local testing against PostgreSQL seems to indicate it works, and doesn't rely on Array#newInstance anymore. Let's see what CI things about it...

yrodiere and others added 11 commits January 6, 2025 16:40
…lans

It's more consistent, and happens to get rid of ArrayMutabilityPlan,
which involved an unnecessary use of Array.newInstance.

I've also seen claims that clone() performs better than
Array.newInstance() due to not having to zero-out the allocated memory,
but I doubt that's relevant here.
No functional impact, it's just less redundant.
The underlying JDBC driver only needs Object[], so there's no need to
try hard to create a T[].
https://hibernate.atlassian.net/browse/HHH-16809

Note these methods only handle arrays of objects
(T[], Object[]) and not arrays of primitives (e.g. int[]),
but that's fine as all the replaced calls were manipulating
arrays of objects. In most cases they were manipulating an array of
JavaType.getJavaTypeClass(),
and that class represents the generic parameter T in JavaType<T>,
which can be many things but never a primitive type,
since generics don't support primitive types.

Co-Authored-By: Yoann Rodière <[email protected]>
…pe to getArrayClass

For consistency with:

1. PrimitiveJavaType#getArrayClass
2. Array#newInstance
… to new Object[]

The dynamic instantiations were originally introduced to fix
the following issues:

* HHH-17201 -- tested in MultiIdEntityLoadTests

The corresponding tests still pass after removing these dynamic array
instantiations.
@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Jan 6, 2025

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [689e749]

› This message was automatically generated.

@yrodiere yrodiere changed the title HHH-16809 + HHH-18976 Add JavaType#createArray HHH-16809 + HHH-18976 Avoid usage of Array.newInstance + Add JavaType#newArray Jan 6, 2025
@yrodiere
Copy link
Member Author

yrodiere commented Jan 9, 2025

Closing in favor of the simpler #9589 . HHH-16809 may be addressed in the future too, but it's not an immediate conceern anymore.

@yrodiere yrodiere closed this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants