On Wed, 3 Jan 2024 12:25:29 GMT, Adam Sotona <asot...@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