On Fri, 15 Nov 2024 17:36:46 GMT, Henry Jen <henry...@openjdk.org> wrote:
>> This PR split out large array/set construction into separate factory methods >> to avoid oversized method trying to construct several of those. >> >> In order to do that, we will need to generate those help methods on demand >> in the class builder. Here we have two approach, one is for dedup set, which >> is processed in advance so we can know what methods should be created. >> >> Another is for random set, such as packages, thus we put those request into >> a queue to amend the class later. >> >> To keep the optimization of caching built value that are references more >> than once, it was implemented using local vars, which doesn't work well for >> helper methods. The existing approach to populate local vars doesn't work >> well with larger scope of split operation, as the slot was allocated on >> lazily built, but the transfer is captured in advance, this count could >> mismatch as built time and run time. >> >> So we make this build in advance, and use a static array for values referred >> more than once. >> >> All the codegen instead of giving index to be loaded, the builder snippet >> now load the wanted set/array to the operand stack to be consistent. > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 13 additional commits since > the last revision: > > - Merge remote-tracking branch 'openjdk/master' into JDK-8321413 > - Minor cleanup > - Rename based on feedback to emphasis building a snippet for loading a > reference > - Fix typo and comment to match latest implementation > - Fix regression failed to setup helper methods properly > - Separate out ModuleDescriptorBuilder and use LoadableArray for paging > - Merge remote-tracking branch 'openjdk/master' into JDK-8321413 > - Loadable support with paging support. The limiting factor is now constant > pool size > - Add more comments, move snippet generation code into standalone class > - Fix typo and add comments based on review feedback > - ... and 3 more: https://git.openjdk.org/jdk/compare/ac9b4cff...4e82d04e src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java line 2: > 1: /* > 2: * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights > reserved. Suggestion: * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java line 149: > 147: clb.withMethodBody(methodName, > 148: methodType(), > 149: isStatic ? ACC_STATIC : ACC_PUBLIC, Suggestion: isStatic ? ACC_STATIC : 0, src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java line 152: > 150: cob -> { > 151: value.emit(cob); > 152: cob.areturn(); You may want this for future compatibility: Suggestion: cob.return_(TypeKind.from(classDesc())); src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 337: > 335: byte[] bytes = generator.genClassBytes(cf); > 336: // Diagnosis help, can be removed > 337: if > (Boolean.parseBoolean(System.getProperty("JlinkDumpSystemModuleClass", > "false"))) { jdk.internal.util is exported to jlink, so you can just use `ClassFileDumper` there. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1878827161 PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1879130112 PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1879128539 PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1879218671
