On Wed, 13 Oct 2021 18:53:22 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> This reimplements core reflection with method handles. >> >> For `Constructor::newInstance` and `Method::invoke`, the new implementation >> uses `MethodHandle`. For `Field` accessor, the new implementation uses >> `VarHandle`. For the first few invocations of one of these reflective >> methods on a specific reflective object we invoke the corresponding method >> handle directly. After that we spin a dynamic bytecode stub defined in a >> hidden class which loads the target `MethodHandle` or `VarHandle` from its >> class data as a dynamically computed constant. Loading the method handle >> from a constant allows JIT to inline the method-handle invocation in order >> to achieve good performance. >> >> The VM's native reflection methods are needed during early startup, before >> the method-handle mechanism is initialized. That happens soon after >> System::initPhase1 and before System::initPhase2, after which we switch to >> using method handles exclusively. >> >> The core reflection and method handle implementation are updated to handle >> chained caller-sensitive method calls [1] properly. A caller-sensitive >> method can define with a caller-sensitive adapter method that will take an >> additional caller class parameter and the adapter method will be annotated >> with `@CallerSensitiveAdapter` for better auditing. See the detailed >> description from [2]. >> >> Ran tier1-tier8 tests. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8013527 >> [2] >> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Minor cleanup. Improve javadoc in CallerSensitiveAdapter Looks a very good simplification. It's great that in the new `poly` benchmarks the regression is so contained (I was frankly expecting more), and I agree with the comments (super interesting discussion btw!) that Poly is probably the most relevant case out there. I noted that in the static case, Poly does regress for fields - do we know why instance Poly is better than static Poly? That seems surprising. src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java line 58: > 56: * Creates a MethodAccessorImpl for a caller-sensitive method. > 57: */ > 58: static MethodAccessorImpl callerSensitiveMethodAccessor(Method > method, MethodHandle dmh) { This method and the one above are identical - they just call `new DirectMethodHandleAccessor` with same parameters. Is the distinction between these two factories still relevant? (besides the different asserts) src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java line 86: > 84: } > 85: > 86: private static final int PARAM_COUNT_MASK = 0x00FF; Is this packing logic required? I get it that it saves footprint - but then you have to always unmask bits to get the argument count (see invoke and other places). If you keep this, it might be worth documenting what the bit layout is supposed to be? src/java.base/share/classes/jdk/internal/reflect/MethodHandleAccessorFactory.java line 151: > 149: var setter = isReadOnly ? null : JLIA.unreflectField(field, > true); > 150: Class<?> type = field.getType(); > 151: if (type == Boolean.TYPE) { dumb question: any reason why `boolean.class` (which is compiled to a LDC) is not used? ------------- Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5027
