On Mon, 5 Jun 2023 23:38:11 GMT, Chen Liang <[email protected]> wrote:
>> This patch aims to improve the performance of MethodTypeDesc in general, as
>> it is currently a performance bottleneck in the Classfile API. A previous
>> revision changed the parameter storage from an array to a list; this is
>> dropped now. Sorry for the force push.
>>
>>
>> Benchmark
>> (descString) Mode Cnt Score Error Units
>> MethodTypeDescFactories.descriptorString
>> (Ljava/lang/Object;Ljava/lang/String;)I avgt 6 27.778 ± 0.573 ns/op
>> MethodTypeDescFactories.descriptorString
>> ()V avgt 6 13.343 ± 0.235 ns/op
>> MethodTypeDescFactories.descriptorString
>> ([IJLjava/lang/String;Z)Ljava/util/List; avgt 6 40.828 ± 0.448 ns/op
>> MethodTypeDescFactories.descriptorString
>> ()[Ljava/lang/String; avgt 6 14.754 ± 0.162 ns/op
>> MethodTypeDescFactories.ofArray
>> (Ljava/lang/Object;Ljava/lang/String;)I avgt 6 8.616 ± 0.132 ns/op
>> MethodTypeDescFactories.ofArray
>> ()V avgt 6 2.146 ± 0.293 ns/op
>> MethodTypeDescFactories.ofArray
>> ([IJLjava/lang/String;Z)Ljava/util/List; avgt 6 14.595 ± 0.235 ns/op
>> MethodTypeDescFactories.ofArray
>> ()[Ljava/lang/String; avgt 6 2.064 ± 0.085 ns/op
>> MethodTypeDescFactories.ofDescriptor
>> (Ljava/lang/Object;Ljava/lang/String;)I avgt 6 97.077 ± 2.482 ns/op
>> MethodTypeDescFactories.ofDescriptor
>> ()V avgt 6 13.563 ± 0.111 ns/op
>> MethodTypeDescFactories.ofDescriptor
>> ([IJLjava/lang/String;Z)Ljava/util/List; avgt 6 130.543 ± 2.847 ns/op
>> MethodTypeDescFactories.ofDescriptor
>> ()[Ljava/lang/String; avgt 6 35.286 ± 0.260 ns/op
>> MethodTypeDescFactories.ofList
>> (Ljava/lang/Object;Ljava/lang/String;)I avgt 6 4.156 ± 0.258 ns/op
>> MethodTypeDescFactories.ofList
>> ()V avgt 6 2.192 ± 0.063 ns/op
>> MethodTypeDescFactories.ofList
>> ([IJLjava/lang/String;Z)Ljava/util/List; avgt 6 41.002 ± 0.235 ns/op
>> MethodTypeDescFactories.ofList
>> ()[Ljava/lang/String; avgt 6 2.200 ± 0.041 ns/op
>
> Chen Liang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixes for review
Thanks for the change and it looks better. I updated the description of
JDK-8309413 to make it clearer.
src/java.base/share/classes/java/lang/constant/MethodTypeDescImpl.java line 57:
> 55: * @param validatedArgTypes {@link ClassDesc}s describing the trusted
> and validated parameter types
> 56: */
> 57: MethodTypeDescImpl(ClassDesc returnType, ClassDesc[]
> validatedArgTypes) {
Can make this constructor private to prevent accidental use with invalidated
argTypes. `MethodTypeDesc.of(ClassDesc returnDesc)` can call `ofTrusted`
factory method.
src/java.base/share/classes/java/lang/constant/MethodTypeDescImpl.java line 106:
> 104: } else {
> 105: result = new MethodTypeDescImpl(returnType,
> ConstantUtils.EMPTY_CLASSDESC);
> 106: }
Suggestion:
int paramCount = types.size() - 1;
var paramTypes = paramCount > 0 ? new ClassDesc[paramCount] :
ConstantUtils.EMPTY_CLASSDESC;
for (int i = 0; i < paramCount; i++) {
paramTypes[i] = ClassDesc.ofDescriptor(types.get(i + 1));
}
result = ofTrusted(returnType, paramTypes);
-------------
PR Review: https://git.openjdk.org/jdk/pull/13186#pullrequestreview-1463810992
PR Review Comment: https://git.openjdk.org/jdk/pull/13186#discussion_r1218713450
PR Review Comment: https://git.openjdk.org/jdk/pull/13186#discussion_r1218736291
