RFR: 8338532: Speed up the ClassFile API MethodTypeDesc#ofDescriptor

[Shaojin Wen]

On Sat, 17 Aug 2024 12:16:03 GMT, Chen Liang <liach at openjdk.org> 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/MethodTypeDescImpl.java line 113:
> 
>> 111:                 || (rightBracket = (descriptor.charAt(1) == ')' ? 1 : descriptor.lastIndexOf(')'))) <= 0
>> 112:                 || (len = descriptor.length() - rightBracket - 1) == 0
>> 113:                 || (len != 1 && len != ConstantUtils.skipOverFieldSignature(descriptor, rightBracket + 1, descriptor.length()))
> 
> Has this be tested against input values like `()A`? I would prefer to fail explicitly with `descriptor.charAt(rightBracket + 1) == 'V'` instead of using `len != 1` and relying on `Wrapper.forBasicType`.

I have tested that `()A` will report an error. If length == 1, rely on Wrapper.forBasicType to detect illegal input.

> src/java.base/share/classes/jdk/internal/constant/MethodTypeDescImpl.java line 125:
> 
>> 123:         int paramCount = 0;
>> 124:         for (int cur = start; cur < end; ) {
>> 125:             int len = ConstantUtils.skipOverFieldSignature(descriptor, cur, end, false);
> 
> I recall skipping over signatures is the main reason descriptor parsing is slow. What is the benchmark result if you allocate a buffer array like `short[] offsets = new short[Math.min(end - start, 255)]` or a `BitSet offsets = new BitSet(end - start)`?

Allocating a buffer will slow down, so I used a long, with each 8 bits storing a parameter length, which can improve performance when the number of parameters is <= 8.

> src/java.base/share/classes/jdk/internal/constant/MethodTypeDescImpl.java line 158:
> 
>> 156:             }
>> 157:             if (len == 0) {
>> 158:                 len = ConstantUtils.skipOverFieldSignature(descriptor, cur, end);
> 
> This can use a simpler version of skip that doesn't validate, like:
> 
> int start = cur;
> while (descriptor.charAt(cur) == '[') {
>     cur++;
> }
> if (descriptor.charAt(cur++) == 'L') {
>     cur = descriptor.indexOf(';', cur) + 1;
> }
> paramTypes[paramIndex++] = ConstantUtils.resolveClassDesc(descriptor, start, cur - start);

Now this version merges the loops, your suggestion doesn't handle paramIndex >= 8, and it won't be faster, the current version of skipOverFieldSignature performs well enough.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720769514
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720452515
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720768233