On Mon, 27 Mar 2023 16:43:17 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy
>> classes and this patch converts it to use Classfile API.
>>
>> Please review.
>>
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional
> commit since the last revision:
>
> StackMapGenerator performance improvements
Keep-alive.
I believe the changes to cache ClassDesc symbol in ClassEntry and avoid
re-computing hash in Utf8Entry can be in a separate patch delivered before the
current migration patch.
For the StackMapGenerator changes, I don't agree with replacing `TOP_TYPE` with
`null`: currently, top type is `0, null, 0`, which can be the default null
instance after valhalla drops (so filling array will no longer be required at
that point). Using `null` makes the code less readable and maintainable as well.
I also question the design to move away from `MethodTypeDesc`: `ofDescriptor`
is expensive, but we can check the slots of a method type with a
`MethodTypeDesc` easily than tokenizing and skipping the raw descriptor string,
like
var slots = type.parameterCount()
for (var param : type.parameterList()) // when parameterList is optimized
if (param.isPrimitive() && (param.descriptorString.charAt(0) == 'D' ||
param.descriptorString.charAt(0) == 'J')) slots++;
Same for processing invoke instructions: if we can reuse the `MethodTypeDesc`
at `invokeInstruction` call sites, we don't need an ad-hoc type computation
either.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/10991#issuecomment-1520993443
