-
-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Remove some reflection usage #14032
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
Remove some reflection usage #14032
Conversation
Motivation: We've been using reflection as a way to abstract over Java API versions. Now that we're based on Java 8, some of this reflection usage is no longer necessary. In some cases, the APIs we were reflecting on are now directly available. In other cases, we'd like a little more performance and can call through method handles instead. Modification: Remove some reflection usage that was necessary when running on Java 6 or 7, and replace it with direct calls if the code is still needed. Replace the more performance sensitive reflection usage with MethodHandles. Result: Cleaner, and perhaps slightly faster, code.
Animal Sniffer inspects bytecode call-sites to look for calls to methods not defined by Java 8. However, it doesn't know about signature polymorphic methods, like MethodHandle.invokeExact(), so we will always get warnings about those calls. Since we can now natively build on Java 8 (this was previously not possible with Java 6), we can instead rely on the test suite to make sure we don't make calls to methods not available in Java 8. The associated animal sniffer issue is mojohaus/animal-sniffer#67
|
Not all reflection usages have been replaced, because 1) reflect gives access to annotations, and method handles don't, and 2) some cases are just not performance sensitive so it's not worth the effort to change code that works. |
franz1981
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to give another round but overall look fine!
|
@chrisvest The allocate uninitialized arrays method, if uses Additionally, I think we can target the |
Oh, you mean we can remove the
The |
c82eca9 to
7816e6b
Compare
|
Lot's of failures in |
Method handles don't work so well on Graal, and lookup may throw InternalError instead of NoSuchMethodException.
This should make our reflection hacks work on Graal.
|
The |
Motivation: In netty#14032 we configured Graal to prepare reflective access to all public methods on the jdk.internal.misc.Unsafe. This apparently includes linking to the getUncompressedObject method, which I think only exist in HotSpot and not in Graal, so the build fails with a linkage error. Modification: Only prepare reflective access for the methods we'll actually call. Result: No more Graal linkage errors during the build.
Motivation: In #14032 we configured Graal to prepare reflective access to all public methods on the jdk.internal.misc.Unsafe. This apparently includes linking to the getUncompressedObject method, which I think only exist in HotSpot and not in Graal, so the build fails with a linkage error. Modification: Only prepare reflective access for the methods we'll actually call. Result: No more Graal linkage errors during the build.
Motivation: The code that detects the availability of allocateUninitializedArray has been failing since netty#14032, because MethodHandle::invokeExact requires exact type matches. Modification: Cast the return value to Object, matching the exact signature of allocateUninitializedArray. The existing cast to byte[] remains and happens immediately after the invocation. Result: Fixes netty#14930
Motivation: The code that detects the availability of `allocateUninitializedArray` has been failing since #14032, because `MethodHandle::invokeExact` requires exact type matches. Modification: Cast the return value to `Object`, matching the exact signature of `allocateUninitializedArray`. The existing cast to `byte[]` remains and happens immediately after the invocation. Result: Fixes #14930
Motivation: The code that detects the availability of `allocateUninitializedArray` has been failing since #14032, because `MethodHandle::invokeExact` requires exact type matches. Modification: Cast the return value to `Object`, matching the exact signature of `allocateUninitializedArray`. The existing cast to `byte[]` remains and happens immediately after the invocation. Result: Fixes #14930
Motivation:
We've been using reflection as a way to abstract over Java API versions. Now that we're based on Java 8, some of this reflection usage is no longer necessary. In some cases, the APIs we were reflecting on are now directly available. In other cases, we'd like a little more performance and can call through method handles instead.
Modification:
Remove some reflection usage that was necessary when running on Java 6 or 7, and replace it with direct calls if the code is still needed. Replace the more performance sensitive reflection usage with MethodHandles.
Also remove the animal sniffer maven plug-in, because animal sniffer does not support signature polymorphic method calls.
See mojohaus/animal-sniffer#67
Result:
Cleaner, and perhaps slightly faster, code.
Fixes #14009