RFR: 8294961: Convert java.base/java.lang.reflect.ProxyGenerator to use the Classfile API to generate proxy classes [v5]

[Chen Liang]

On Wed, 3 Jan 2024 12:25:29 GMT, Adam Sotona <asotona at openjdk.org> wrote:

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

Turns out your approach to avoid MTD here is apparently useless; `MethodTypeDesc` is still created for initializing the local tracker `topLocal` in `DirectCodeBuilder`. In addition, `StackMapDecoder` also uses `methodTypeSymbol` to compute the initial frame.

IMO we should just stay with MTD; the descriptor breakdown happens too often and, from previous benchmarks, descriptor breakdown is actually slow (which gives CF API a small edge over ASM here). But we can still replace the `parameterList()` iteration with index-based iteration to avoid array copies.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17121#discussion_r1444049504