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

[Christian Hagedorn]

On Wed, 15 Oct 2025 08:57:02 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 four additional commits since the last revision:
> 
>  - Add run without fixed stress seed
>  - Reorder flags
>  - Remove unnecessary CompileCommand=dontinline
>  - Change name

Few more nits but otherwise, looks good, thanks for the updates!

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

> 393: 
> 394: // ------------------------------------------------------------------
> 395: // ciInstanceKlass::get_non_static_field_by_offset

I think you can remove this - we used to add it in the early days but not anymore.

Suggestion:

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

> 397:   for (int i = 0, len = nof_nonstatic_fields(); i < len; i++) {
> 398:     ciField* field = _nonstatic_fields->at(i);
> 399:     int  field_off = field->offset_in_bytes();

Suggestion:

    ciField* field = _nonstatic_fields->at(i);
    int field_off = field->offset_in_bytes();

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

> 436: // ------------------------------------------------------------------
> 437: // ciInstanceKlass::get_field_type_by_offset
> 438: //

Suggestion:

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

> 438: //
> 439: // This is essentially a shortcut for:
> 440: //  get_field_by_offset(field_offset, is_static)->layout_type()

Suggestion:

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

test/hotspot/jtreg/compiler/loopopts/TestVerifyLoopOptimizationsHitsMemLimit.java line 32:

> 30:  *          This is caused by the high number of verification passes triggered
> 31:  *          in PhaseIdealLoop::split_if_with_blocks_post and repetitive memory
> 32:  *          allocations while building the ideal Loop tree in preparation for

Suggestion:

 *          allocations while building the ideal loop tree in preparation for

test/hotspot/jtreg/compiler/loopopts/TestVerifyLoopOptimizationsHitsMemLimit.java line 49:

> 47:  *      compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit
> 48:  * @run main compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit
> 49:  *

Suggestion:

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27731#pullrequestreview-3428809389
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499445193
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499440534
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499447501
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499449135
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499458403
PR Review Comment: https://git.openjdk.org/jdk/pull/27731#discussion_r2499454835