RFR: 8264288: Performance issue with MethodHandle.asCollector [v2]

[Jorn Vernee]

On Fri, 2 Apr 2021 13:56:31 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   - Address review comments
>>   - Use cached version of store func getter
>>   - Use ARRAY_STORE intrinsic for array stores
>>   - Generate direct call to Array.newInstance instead of using an array constructor handle
>>   - Intrinsify call to Array.newInstance if the component type is constant
>
> src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java line 874:
> 
>> 872:                 case NEW_ARRAY:
>> 873:                     Class<?> rtype = name.function.methodType().returnType();
>> 874:                     if (isStaticallyNameable(rtype)) {
> 
> Why do you remote `isStaticallyNameable(rtype)` check? 
> 
> It is there for a reason: only a very limited set of classes can be directly referenced from LF bytecode.

I removed the `NEW_ARRAY` intrinsic altogether at first, but added it back in the latest commit. I didn't add the check since I was not aware of the restriction.

Good to know, will add a check.

> src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1897:
> 
>> 1895:         // field of the receiver handle.
>> 1896:         // This is left as a followup enhancement, as it needs to be investigated if this causes
>> 1897:         // profile pollution.
> 
> Profile pollution shouldn't be a problem here: both types (element type + array type) will be constants attached to the "receiver" BMH instance and during inlining will be fetched from there.  
> 
> I'm fine with handling it as a separate RFE.

That's what I thought as well (but not 100% sure). I can partially roll back the last commit so the code still uses an injected array constructor handle, and then it should be easy to add caching in the cases where the component type is a reference type.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3306