RFR: 8366990: C2: Compilation hits the memory limit when verifying loop opts in Split-If code [v9]

[Damon Fenacci]

On Thu, 6 Nov 2025 15:40:59 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

>> This PR prevents the C2 compiler from hitting memory limits during compilation when using `-XX:+StressLoopPeeling` and `-XX:+VerifyLoopOptimizations` in certain edge cases. The fix addresses an issue where the `ciEnv` arena grows uncontrollably due to the high number of verification passes, a complex IR graph, and repeated field accesses leading to unnecessary memory allocations.
>> 
>> ### Analysis
>> 
>> This issue was initially detected with the fuzzer. The original test from the fuzzer was reduced
>> and added to this PR as a regression test.
>> 
>> The test contains a switch inside a loop, and stressing the loop peeling results in
>> a fairly complex graph.  The split-if optimization is applied agressively, and we
>> run a verification pass at every progress made.
>> 
>> We end up with a relatively high number of verification passes, with each pass being
>> fairly expensive because of the size of the graph.
>> Each verification pass requires building a new `IdealLoopTree`. This is quite slow
>> (which is unfortunately hard to mitigate), and also causes inefficient memory usage
>> on the `ciEnv` arena.
>> 
>> The inefficient usages are caused by the `ciInstanceKlass::get_field_by_offset` method.
>> At every call, we have
>> - One allocation on the `ciEnv` arena to store the returned `ciField`
>> - The constructor of `ciField` results in a call to `ciObjectFactory::get_symbol`, which:
>>   - Allocates a new `ciSymbol` on the `ciEnv` arena at every call (when not found in `vmSymbols`)
>>   - Pushes the new symbol to the `_symbols` array
>> 
>> The `ciEnv` objects returned by `ciInstanceKlass::get_field_by_offset` are only used once, to
>> check if the `BasicType` of a static field is a reference type.
>> 
>> In `ciObjectFactory`, the `_symbols` array ends up containg a large number of duplicates for certain symbols
>> (up to several millions), which hints at the fact that `ciObjectFactory::get_symbol` should not be called
>> repeatedly as it is done here.
>> 
>> The stack trace of how we get to the `ciInstanceKlass::get_field_by_offset` is shown below:
>> 
>> 
>> ciInstanceKlass::get_field_by_offset ciInstanceKlass.cpp:412
>> TypeOopPtr::TypeOopPtr type.cpp:3484
>> TypeInstPtr::TypeInstPtr type.cpp:3953
>> TypeInstPtr::make type.cpp:3990
>> TypeInstPtr::add_offset type.cpp:4509
>> AddPNode::bottom_type addnode.cpp:696
>> MemNode::adr_type memnode.cpp:73
>> PhaseIdealLoop::get_late_ctrl_with_anti_dep loopnode.cpp:6477
>> PhaseIdealLoop::get_late_ctrl loopnode.cpp:6439
>> PhaseIdealLoop::build_lo...
>
> Benoît Maillard has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply suggestions from code review
>   
>   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>

Looks good to me too. Thanks @benoitmaillard!

src/hotspot/share/ci/ciInstanceKlass.cpp line 435:

> 433: 
> 434: // This is essentially a shortcut for:
> 435: //   get_field_by_offset(field_offset, is_static)->layout_type()

Suggestion:

// get_field_by_offset(field_offset, is_static)->layout_type()


Did you actually want the indentation?

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

Marked as reviewed by dfenacci (Committer).

PR Review: https://git.openjdk.org/jdk/pull/27731#pullrequestreview-3429025686
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499625670