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

[Chen Liang]

On Sat, 17 Aug 2024 12:25:58 GMT, Shaojin Wen <duke at openjdk.org> wrote:

>> 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.

We just exposed an internal exception behavior to public. Might add in wrapper.forBasicType that this IAE is part of public API.

>> 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.

Hmm? I mean this block can be a new method (the cur pointer moving part) and replace existing skipOver call that does extra validation

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720770934
PR Review Comment: https://git.openjdk.org/jdk/pull/20611#discussion_r1720771392