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

[Tobias Hartmann]

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 agree that such exceptional cases are usually not worth optimizing but in this case we already emitted all the code to handle them, so why deopt? The change looks good to me.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25149#pullrequestreview-2836166665