On Mon, 23 Dec 2024 06:15:58 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 incrementally with one additional
> commit since the last revision:
>
> Address review comments
This looks good to me.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java line 346:
> 344: * ...
> 345: * ar[pageSize-1] = elements[pageSize - 1];
> 346: * methodNamePrefix1(ar);
Suggestion:
* methodNamePrefix_1(ar);
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/Snippets.java line 397:
> 395: }
> 396:
> 397: // each helper function is T[] methodNamePrefix{pageNo}(T[])
Suggestion:
// each helper function is T[] methodNamePrefix_{pageNo}(T[])
test/jdk/tools/jlink/JLink20000Packages.java line 106:
> 104: "--module-source-path", src.toString(),
> 105: "--module", "bug8321413x"
> 106: });
FYI. You can consider using `jdk.test.lib.compiler.CompilerUtils::compile`
method to invoke javac in-process.
test/jdk/tools/jlink/SnippetsTest.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.
-------------
Marked as reviewed by mchung (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/21022#pullrequestreview-2528467896
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1901409464
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1901409680
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1901412049
PR Review Comment: https://git.openjdk.org/jdk/pull/21022#discussion_r1901412151
