RFR: 8350607: Consolidate MethodHandles::zero into MethodHandles::constant [v2]

Chen Liang liach at openjdk.org
Tue Mar 4 14:15:00 UTC 2025


On Tue, 4 Mar 2025 11:56:50 GMT, Jorn Vernee <jvernee at 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


More information about the core-libs-dev mailing list