On Wed, 26 Apr 2023 16:00:07 GMT, Chen Liang <li...@openjdk.org> wrote:
> 1. CodeBuilder.invokeInstruction and CodeBuilder.fieldInstruction can pass
> their symbols to the NameAndTypeEntry as well, since it's almost always going
> to be used by stack map generation later.
Yes, they actually do. CodeBuilder conventional methods delegate down to the
NaTEntry construction from symbols. I hope I didn't miss any important path.
> 2. Since the casts to access the field and method type symbols in
> `NameAndTypeEntryImpl` is quite complex, can we move it to a utility method
> in `Util` for cleaner call sites?
> 3. `parameterSlots` `parseParameterSlots` `maxLocals` in `Util` can probably
> operate on a `MethodTypeDesc` than a bitset, especially that the method type
> symbols are available in most scenarios already.
> 4. `StackMapGenerator.processInvokeInstructions` can probably scan through
> the `MethodTypeDesc` instead of using the hand-rolled `while (++pos < descLen
> && (ch = mDesc.charAt(pos)) != ')')` loop. In fact, the whole loop can be
> replaced by `Util.parameterSlots()`
> 5. `StackMapGenerator.isDoubleSlot(ClassDesc)` should be moved to `Util` and
> used by `parameterSlots()` etc (assuming they've migrated to
> `MethodTypeDesc`), and implementation be updated to:
>
> ```java
> public static boolean isDoubleSlot(ClassDesc desc) {
> return desc.isPrimitive() && (desc.charAt(0) == 'D' || desc.charAt(0)
> == 'J');
> }
> ```
>
> to avoid potentially slow string comparisons.
These are great suggestions, I'll work on them.
Thanks!
> If we can hash internal names for constant pool without calling
> Util.toInternalName on ClassDesc instances, can that offset the performance
> penalty of toInternalName?
Yes, that may help. There is already manual hash calculation from the raw
bytes, so it should work the same way.
Good idea, thanks.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1525075958
PR Comment: https://git.openjdk.org/jdk/pull/13671#issuecomment-1527577715
