RFR: 8337225: Demote maxStack and maxLocals from CodeModel to CodeAttribute
Chen Liang
liach at openjdk.org
Fri Jul 26 13:19:32 UTC 2024
On Fri, 26 Jul 2024 08:03:01 GMT, Adam Sotona <asotona at openjdk.org> wrote:
>> As discussed in offline meeting, the max stack and locals information are part of the code attribute and not meaningful for buffered code elements. Computation would be costly and these see no real usage during transformations. Thus, the proposed solution is to move these APIs to be CodeAttribute specific, as this is already how all these APIs' users are using.
>>
>> Also removed useless `Writable` on buffered models, and fixed `BufferedMethodBuilder::code` implementation.
>
> src/java.base/share/classes/jdk/internal/classfile/impl/BufferedCodeBuilder.java line 67:
>
>> 65: this.maxLocals = Util.maxLocals(methodInfo.methodFlags(), methodInfo.methodTypeSymbol());
>> 66: if (original != null)
>> 67: this.maxLocals = Math.max(this.maxLocals, original.maxLocals());
>
> `original::maxLocals`set the counter for `CodeBuilder::allocateLocal`
> By restricting calculation of maxLocals to "origin instanceof CodeAttribute" may cause invalid locals allocations for chained code builders. The number might not be exposed in the API, however we need to know it internally.
I think we should just do `Util.maxLocals` without this `origin` check. Your said problem can happen too if a DirectMethodBuilder transforms a BufferedCodeBuilder.Model.
Note that any code builder can receive a code model half way with `cob.transform(anyModel, CodeTransform.ACCEPT_ALL)`, which the buffered code builder initialization also uses.
I think we should just ask users to use `CodeLocalsShifter` in that case, especially if the transform is inserting new locals. We cannot buffer existing chained builders for performance reasons.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20338#discussion_r1693049481
More information about the core-libs-dev
mailing list