Less reflection and move to MethodHandles in Hibernate Search extension #15152
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Yoann Rodière <[email protected]>
We don't need to, because: 1. We perform all reflection operations, except getting values from entities, at static init. 2. After static init, we hold references to all Methods/Fields that we will actually use at runtime. SubstrateVM is able to detect those and to properly build a native image that will handle calling invoke() on these Methods/Fields. Signed-off-by: Yoann Rodière <[email protected]>
… Hibernate Search with GraalVM 21+ GraalVM 21.0 supports method handles, so let's use them. The main benefit is that we'll use them in JVM mode too, where they should theoretically perform slightly better. Signed-off-by: Yoann Rodière <[email protected]>
|
Note I tested this patch locally by running the Hibernate Search integration tests with OpenJDK, GraalVM 20.3 (with |
gsmet
approved these changes
Feb 18, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We don't need to enable runtime reflection on user classes, because:
at static init.
will actually use at runtime. SubstrateVM is able to detect those and
to properly build a native image that will handle calling invoke() on
these Methods/Fields.
Also, when using GraalVM 21 or OpenJDK, we can move from
java.lang.reflecttoMethodHandles. Which is what Hibernate Search uses by default, and what we tested in most depth.This supersedes #14739: I added conditional behavior so that we use
java.lang.reflecton GraalVM 20 and below, butMethodHandleson OpenJDK and GraalVM 21+. The original PR was always usingMethodHandles, which could be a problem with GraalVM 20 and below. I also added specific tests to check that property access works as expected.Related: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/MethodHandles.20in.20Hibernate.20Search