RFR: 8308899: Introduce Classfile.Context and improve Classfile.Option(s) [v14]

[Chen Liang]

On Mon, 12 Jun 2023 15:55:49 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Classfile context object and multi-state options have been discussed at https://mail.openjdk.org/pipermail/classfile-api-dev/2023-May/000321.html
>> This patch implements the proposed changes in Classfile API and fixes all affected code across JDK sources and tests.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 38 commits:
> 
>  - Merge branch 'master' into JDK-8308899-context
>  - Revert of DirectCodeBuilder.needsStackMap pre-calculation
>  - Revert "removal of ClassHierarchyImpl.DEFAULT_RESOLVER"
>    esolver.java
>  - Revert "proposed semi-synchronized caching, where the map is not locked during delegate call"
>    
>    This reverts commit ae2877512d978468743bcaa7e0f596729f12ee72.
>  - fixed StackMapsOption dispatching in DirectCodeBuilder
>  - proposed semi-synchronized caching, where the map is not locked during delegate call
>  - used Factory.INSTANCE for system ClassHierarchyResolver cache
>  - Revert "ClassHierarchyResolver::ofSystem is now thread-unsafe"
>    
>    This reverts commit f3099cd5b252924392995bf65edc710c27822d2b.
>  - ClassHierarchyResolver::ofSystem is now thread-unsafe
>  - removal of ClassHierarchyImpl.DEFAULT_RESOLVER
>    introduction of ClassHierarchyResolver::ofSystem factory method
>    ClassfileImpl does not pre-initialize ClassHierarchyResolverOption with default
>  - ... and 28 more: https://git.openjdk.org/jdk/compare/5d5ae352...9e0d141e

I have found the cause of the regression: there's redundant stack map generation in DirectCodeBuilder for methods with trivial stack map table (null).

src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java line 334:

> 332:                     maxLocals = original.maxLocals();
> 333:                     maxStack = original.maxStack();
> 334:                     stackMapAttr = original.findAttribute(Attributes.STACK_MAP_TABLE).orElse(null);

On the baseline, if a stack map table attribute can be reused but this findAttribute doesn't find an attribute (implying an implicit stack map, potentially for simple code?), we will not perform a stack map generation and simply reuse a trivial stack map (null), which should be a correct algorithm.

src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java line 349:

> 347:             }
> 348: 
> 349:             private boolean writeOriginalAttribute(BufWriterImpl buf) {

As a result (see previous comment), we will run into a redundant stack map generation when the original stack map table is trivial (absent). Maybe we want to consider user-configured selective stack map regeneration to replace this faulty logiic.

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

PR Review: https://git.openjdk.org/jdk/pull/14180#pullrequestreview-1476665044
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1227803483
PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1227808744