RFR: 8349191: Test compiler/ciReplay/TestIncrementalInlining.java failed

[Christian Hagedorn]

On Wed, 6 Aug 2025 09:14:36 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

> This PR fixes a bug caused by synchronization issues in the print inlining system. Individual segments of a single line of output are interleaved with output from other commpile threads, causing tests that parse replay files to fail.
> 
> A snippet of a problematic replay file is shown below:
> 
> <writer thread='55214'/>
>                             @ 0   compiler.ciReplay.IncrementalInliningTest::level0 (4 bytes)   force inline by annotation
>                               @ 0   compiler.ciReplay.IncrementalInliningTest::level1 (4 bytes)   inline (hot)
>                                 @ 0   compiler.ciReplay.IncrementalInliningTest::level2 (4 bytes)
> <writer thread='55217'/>
> <make_not_compilable thread='55217' osr='0' level='4' reason='excluded by CompileCommand' method='java.lang.Class isPrimitive ()Z' bytes='5' count='1536' iicount='1536' stamp='0.110'/>
> <writer thread='55214'/>
>    force inline by annotation
>                                   @ 0   compiler.ciReplay.IncrementalInliningTest::late (4 bytes)   force inline by annotation   late inline succeeded
>                                     @ 0   compiler.ciReplay.IncrementalInliningTest::level4 (6 bytes)   failed to inline: inlining too deep
> 
> 
> This makes the output impossible to parse for tests like `compiler/ciReplay/TestIncrementalInlining.java`, as they rely on regular expressions to parse individual lines. Because it is a synchronization issue, the bug quite intermittent and I was only able to reproduce it with mach5 in tier 7.
> 
> This bug was caused by [JDK-8319850](https://bugs.openjdk.org/browse/JDK-8319850), as it introduced important changes in the print inlining system. With these changes,  individual segments of the output are printed directly to tty, and this risks causing problematic interleavings with multiple compile threads. 
> 
> My proposed solution is to simply print everything to a `stringStream` first, and then dump it to `tty`. The PR also removes the relevant tests from `ProblemList.txt`.
> 
> ### Testing
> - [x] [GitHub Actions](https://github.com/benoitmaillard/jdk/actions?query=branch%3AJDK-8349191)
> - [x] tier1-3, plus some internal testing
> - [x] tier7 for the relevant tests (`TestIncrementalInlining.java` and `TestInliningProtectionDomain.java`)
> 
> Thanks for reviewing!

Looks good, thanks for deep diving into this in order to revive the test!

src/hotspot/share/opto/printinlining.cpp line 52:

> 50:   stringStream ss;
> 51:   _root.dump(&ss, -1);
> 52:   tty->print_raw(ss.freeze());

General thought: I see that we use the proposed pattern to print a `stringStream` in existing code but also a different pattern with `as_string()`:
https://github.com/openjdk/jdk/blob/c56fb0b6eff7d3f36bc65f300b784e0dd73c563e/src/hotspot/share/opto/compile.cpp#L614

Can anybody comment on which one that should be preferred?

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26654#pullrequestreview-3096083746
PR Review Comment: https://git.openjdk.org/jdk/pull/26654#discussion_r2259591574