On Sun, 24 Dec 2023 03:14:44 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> minor StackCounter fix
>
> src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 737:
>
>> 735: private void generateMethod(ClassBuilder clb, ClassEntry
>> className) {
>> 736: var cp = clb.constantPool();
>> 737: var desc = new StringJoiner("", "(", ")" +
>> returnType.descriptorString());
>
> We should just use an MTD here; the MTD will be passed to StackCounter so we
> don't have to recompute a MTD.
>
> The MT to MTD conversion shouldn't be too costly; the overhead probably comes
> from Optional, which we have to wait until Valhalla, as proxy generation is
> unlikely to be hot and compiled by C2.
Original code has been significant in profiler.
MethodTypeDesc desc = MethodTypeDesc.of(toClassDesc(returnType),
Arrays.stream(parameterTypes).map(ProxyGenerator::toClassDesc).toArray(ClassDesc[]::new));
1. each `toClassDesc` builds `descriptorString` and parses/validates it while
constructing `ClassDesc`
2. `Arrays.stream(...).map(...).toArray(...)` allocates an array
3. `MethodTypeDesc.of(...)` clones the array and iterates params to check for
void
4. `desc.descriptorString()` then finally use the `StringJoiner`
Optimized code only joins `descriptorString`, no validations, no streaming, no
arrays, no cloning.
I suggest this patch as this code is considered as performance critical.
However we can go through `ClassDesc` and `MethodTypeDesc` if not performance
critical or if the conversions would be optimized.
For example better (trusted) paths from `MethodType` to `MethodTypeDescriptor`
and from `Class` to `ClassDesc`, avoiding at least validations.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17121#discussion_r1440395021
