RFR: 8353638: C2: deoptimization and re-execution cycle with StringBuilder

[Marc Chevalier]

On Fri, 9 May 2025 14:57:54 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

> Unlike what was assumed at first, it is quite different from [JDK-8346989](https://bugs.openjdk.org/browse/JDK-8346989). The problem is actually unrelated to `StringBuilder`, but has to do with the underlying array allocation. 
> 
> Here, the problem is that the array allocation function, that is throwing when given a negative length, causes a deopt rather than using the compiled exception handlers. This is an old workaround, and the flag `StressCompiledExceptionHandlers` to rather use compiled handlers instead of deopting was added in [JDK-8004741](https://bugs.openjdk.org/browse/JDK-8004741) in 2012. This flag is used in testing since october 2022.
> 
> So maybe it's time to use the compiled exception handlers! I propose to turn them on by default, and instead, add a diagnostic flag to deopt instead, in case something goes wrong. Doing so improve the performance to match the ones of C1 (both for direct array allocation, and `StringBuilder` construction). For instance, with the case given in the JBS issue:
> 
> Stop at level 0
> CompileCommand: compileonly C.test* bool compileonly = true
> 
> real	0m4,277s
> user	0m4,214s
> sys	0m0,117s
> 
> Stop at level 1
> CompileCommand: compileonly C.test* bool compileonly = true
> 
> real	0m4,104s
> user	0m4,079s
> sys	0m0,106s
> 
> Stop at level 2
> CompileCommand: compileonly C.test* bool compileonly = true
> 
> real	0m4,308s
> user	0m4,239s
> sys	0m0,145s
> 
> Stop at level 3
> CompileCommand: compileonly C.test* bool compileonly = true
> 
> real	0m4,304s
> user	0m4,247s
> sys	0m0,132s
> 
> Default (Stop at level 4)
> CompileCommand: compileonly C.test* bool compileonly = true
> 
> real	0m4,086s
> user	0m4,059s
> sys	0m0,122s
> 
> 
> 
> I've run some tests (up to tier10), it seems all fine, ignoring the usual noise. I've checked with @dougxc, it shouldn't impact Graal as it doesn't use `OptoRuntime`.

I've found a failing test (maybe even two) related to asynchronous exceptions when using this flag. I suspect something is wrong there ([JDK-8356648](https://bugs.openjdk.org/browse/JDK-8356648)). It's also not clear to me whether the failure is directly linked to faulty exceptions handlers: asynchronous exceptions have a complicated (to me) handshake mechanism and maybe it's faulty without the deopts (or deopts makes it less likely). Unlike in [JDK-8004741](https://bugs.openjdk.org/browse/JDK-8004741) (that added `StressCompiledExceptionHandlers`), I'm not hitting the assert "missing exception handler" (that still exists). Aside from being guarded by the same flag, the relation is not clear to me.

For allocations, I couldn't find any problem, and the logic seems simpler. I think it's fine to still use the one that works.

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

PR Comment: https://git.openjdk.org/jdk/pull/25149#issuecomment-2877316276