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

[Benoît Maillard]

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

I have made the following (significant) changes that are ready for review:
- Replaced the test method with a further reduced version that now takes a little more than one second compared to  ~40s previously
- Added a second run without a fixed stress seed (as the compilation is now fast enough)
- Added a memory limit of `100M`

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

PR Comment: https://git.openjdk.org/jdk/pull/27731#issuecomment-3405314464