On Sun, 18 Aug 2024 21:27:44 GMT, Claes Redestad <[email protected]> wrote:
>> The current implementation of ofDescriptor puts return type and parameter
>> types together in an ArrayList, and then splits them into return type and
>> array of parameter types. This ArrayList creation is unnecessary,
>> considering most descriptors only have few parameter types.
>>
>> By splitting return type and parameter types separately and scanning the
>> descriptor first to get the number of parameters, we can just allocate an
>> exact, trusted array for the resulting MethodTypeDesc without copy.
>
> src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java line 309:
>
>> 307: private static ClassDesc resolveClassDesc(String descriptor, int
>> start, int len) {
>> 308: if (len == 1) {
>> 309: return
>> Wrapper.forPrimitiveType(descriptor.charAt(start)).basicClassDescriptor();
>
> This was already piggy-backing on a quite fast routine. If it's a clean,
> significant win then I'm fine with this (untangling `Wrapper` from this is
> probably good, all things considered), but then we should also clean up
> `Wrapper` to not carry some descriptors around.
I have removed the Wrapper.forPrimitiveType(char) method. It seems that this is
the only method that can be removed. Other descriptors related methods still
need to be used.
> src/java.base/share/classes/jdk/internal/constant/MethodTypeDescImpl.java
> line 137:
>
>> 135: var returnType = resolveClassDesc(descriptor, rightBracket + 1,
>> retTypeLength);
>> 136: if (length == 3 && returnType == CD_void) {
>> 137: return Constants.MTD_void;
>
> Feels a bit like overfitting with quite limited data.
>
> Could this use `ConstantDescs.MTD_void` instead or does that cause a
> bootstrap cycle?
Using ConstantDescs.MTD_void can also solve the bootstrap cycle, good idea, I
have fixed it
> test/micro/org/openjdk/bench/java/lang/classfile/MethodTypeDescBench.java
> line 23:
>
>> 21: * questions.
>> 22: */
>> 23: package org.openjdk.bench.java.lang.classfile;
>
> Wrong package as the code being tested is in java.lang.constant. Also there
> is already a related microbenchmark in
> org.openjdk.bench.java.lang.constant.MethodTypeDescFactories - can you check
> if that covers your needs?
MethodTypeDescFactories has some illegal MethodTypeDesc, and throwing
exceptions will affect the performance comparison of normal scenarios.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1721080326
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1721064527
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720100307
