On Tue, 4 Mar 2025 11:56:50 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> We no longer load DelegateMH as we no longer rebind
>
> src/java.base/share/classes/java/lang/invoke/LambdaForm.java line 1658:
>
>> 1656: var carrier = argument(0, L_TYPE).withConstraint(species); //
>> BMH bound with data
>> 1657: Name[] constNames = new Name[] { carrier, new
>> Name(species.getterFunction(0), carrier) };
>> 1658: return LF_constant[type.ordinal()] = create(1, constNames,
>> Kind.CONSTANT);
>
> I think this caching logic should be in `constantForm`, which also does the
> cache lookup.
Putting the field write in the `create` method is to help reduce the code that
JIT needs to compile by reducing the getter's code size.
> src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 972:
>
>> 970: callFilter = null;
>> 971: else
>> 972: callFilter = new Name(LambdaForm.identity(newType),
>> newType.btWrapper.zero());
>
> Maybe we could have a `Name` constructor that just takes an `Object` and
> returns it like this.
I don't think that is necessary - the majority of constant values like this are
immediately sent to a NamedFunction, which already accepts such constants
natively. The only case such a Name is necessary is for LF return values.
> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestDynamicRegenerateHolderClasses.java
> line 42:
>
>> 40: static String CHECK_MESSAGES[] = {"java.lang.invoke.Invokers$Holder
>> source: shared objects file (top)",
>> 41:
>> "java.lang.invoke.DirectMethodHandle$Holder source: shared objects file
>> (top)",
>> 42:
>> "java.lang.invoke.DelegatingMethodHandle$Holder source: shared objects file
>> (top)",
>
> I don't understand why this change is needed. Could you please explain it?
With the simplification of `MethodHandles.constant`, it seems invoking
`MethodHandles.constant` no longer need to initialize `DelegatingMethodHandle`
because there is no longer need to rebind some MethodHandle; anyways we are
loading fewer classes now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23706#discussion_r1979534142
PR Review Comment: https://git.openjdk.org/jdk/pull/23706#discussion_r1979537500
PR Review Comment: https://git.openjdk.org/jdk/pull/23706#discussion_r1979543546
