On Fri, 13 Dec 2024 19:30:19 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:
> 
>   Move up Snippet setup as a builder

This version looks much better.

I like the idea to introduce a Snippet class to emit code to create a Set, an 
array, load a constant, an enum constant, etc.  It would split the method into 
helper methods if it exceeds certain threshold.   Naming is hard and this 
version is much easier to understand. 

Some has javadoc and some does not.  It'd be good to add javadoc to 
Snippets.java for completeness.

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

PR Review: https://git.openjdk.org/jdk/pull/21022#pullrequestreview-2503386683

Reply via email to