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

[Chen Liang]

On Mon, 16 Sep 2024 17:50:52 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.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 669:

> 667:                         CLASS_INIT_NAME,
> 668:                         MTD_void,
> 669:                         ACC_PUBLIC | ACC_STATIC,

Suggestion:

                        ACC_STATIC,

By convention, we simply set the static flag without anything else set.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 1248:

> 1246:                             methodName,
> 1247:                             MethodTypeDesc.of(CD_EXPORTS.arrayType()),
> 1248:                             ACC_PRIVATE | ACC_FINAL | ACC_STATIC,

Just curious, why do we mark our split provider methods final? The final flag is only used for hiding.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 1427:

> 1425:                     var methodName = "module" + index + "Packages";
> 1426:                     addModuleHelpers(clb -> {
> 1427:                     clb.withMethodBody(

We should indent this lambda.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 1598:

> 1596: 
> 1597:             int counterStoredValues = 0;
> 1598:             ClassDesc owner;

Suggestion:

            final ClassDesc owner;

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java line 1804:

> 1802: 
> 1803:         static <T extends Enum<T>> BiConsumer<CodeBuilder, T> getEnumLoader(ClassDesc enumClassDesc) {
> 1804:             return (cob, element) -> cob.getstatic(enumClassDesc, element.toString(), enumClassDesc);

Suggestion:

            return (cob, element) -> cob.getstatic(enumClassDesc, element.name(), enumClassDesc);

test/jdk/tools/jlink/JLink3500Packages.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.

test/jdk/tools/jlink/JLink3500Packages.java line 35:

> 33: /*
> 34:  * @test
> 35:  * @summary Make sure that 4000 packages in a uber jar can be linked using jlink.

Should we mention that the number 3500 or 4000 is an arbitrary large number greater than `ModuleDescriptorBuilder.SET_SIZE_THRESHOLD` to trigger the provider method generation?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776117490
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776117924
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776110978
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776113761
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776114256
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776123965
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1776108517