RFR: 8321413: IllegalArgumentException: Code length outside the allowed range while creating a jlink image [v11]

[Chen Liang]

On Fri, 15 Nov 2024 17:36:46 GMT, Henry Jen <henryjen at 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