On Mon, 12 Jun 2023 15:55:49 GMT, Adam Sotona <asot...@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
