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