RFR: 8267532: C2: Profile and prune untaken exception handlers [v10]

[Vladimir Kozlov]

I hit is too on my Mac, I filed following bug and assigned to Jorn.

https://bugs.openjdk.org/browse/JDK-8321141

Note, I checked and all testing passed when 8267532 was reviewed.
May be something to do with old Xcode I used to compile or something else.

Thanks,
Vladimir K

On 11/30/23 4:47 AM, Vitaly Provodin wrote:
> Hi all,
> 
> With the latest changes I got the following error
> 
> =======================8<----------------------
> ./src/hotspot/share/ci/ciMethodData.cpp:477:1: error: non-void function does not return a value in all control paths 
> [-Werror,-Wreturn-type]
> }
> ^
> 1 error generated.
> make[3]: *** 
> [/opt/teamcity-agent/work/602288ed8ca22f30/build/macosx-aarch64-server-release/hotspot/variant-server/libjvm/objs/ciMethodData.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [hotspot-server-libs] Error 2
> make[2]: *** Waiting for unfinished jobs….
> =======================8<———————————
> 
> Here is my build environment
> 
> Configuration summary:
> * Name: macosx-aarch64-server-release
> * Debug level: release
> * HS debug level: product
> * JVM variants: server
> * JVM features: server: 'cds compiler1 compiler2 dtrace epsilongc g1gc jfr jni-check jvmci jvmti management parallelgc 
> serialgc services shenandoahgc vm-structs zgc'
> * OpenJDK target: OS: macosx, CPU architecture: aarch64, address length: 64
> * Version string: 22+9-b1917 (22)
> * Source date: 1701334649 (2023-11-30T08:57:29Z)
> 
> Tools summary:
> * Boot JDK: openjdk version "22" 2024-03-19 OpenJDK Runtime Environment JBR-22+9-1795-nomod (build 22+9-b1795) OpenJDK 
> 64-Bit Server VM JBR-22+9-1795-nomod (build 22+9-b1795, mixed mode)
> * Toolchain: clang (clang/LLVM from Xcode 12.2)
> * Sysroot: /Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk
> * C Compiler: Version 12.0.0 (at /usr/bin/clang)
> * C++ Compiler: Version 12.0.0 (at /usr/bin/clang++)
> 
> Could you please clarify how to overcome this issue?
> 
> Thanks,
> Vitaly
> 
> 
>> On 27. Nov 2023, at 17:04, Jorn Vernee <jvernee at openjdk.org> wrote:
>>
>> On Thu, 23 Nov 2023 15:31:28 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
>>
>>>> The issue is essentially that for the Java try-with-resource construct, javac generates multiple calls to `close(`) 
>>>> the resource. One of those calls is inside the hidden exception handler of the try block. The issue for us is that 
>>>> typically the exception handler is never entered (since no exception is thrown), however we don't profile exception 
>>>> handlers at the moment, so the block is not pruned. C2 doesn't inline the `close()` call in the handler due to low 
>>>> call site frequency. As a result, the receiver of that call escapes and can not be scalar replaced, which then leads 
>>>> to a loss in performance.
>>>>
>>>> There has been some discussion on the JBS issue that this could be fixed by profiling catch blocks. And another 
>>>> suggestion that partial escape analysis could help here to prevent the object from escaping. But, I think there are 
>>>> other benefits to being able to prune dead catch blocks, such as general reduction in code size, and other 
>>>> optimizations being possible by dead code being eliminated. So, I've implemented catch block profiling + pruning in 
>>>> this patch.
>>>>
>>>> The implementation is essentially very straightforward: we allocate an extra bit of profiling data for each
>>>> exception handler of a method in the `MethodData` for that method (which holds all the profiling
>>>> data). Then when looking up the exception handler after an exception is thrown, we mark the
>>>> exception handler as entered. When C2 parses the exception handler block, and it sees that it has
>>>> never been entered, we emit an uncommon trap instead.
>>>>
>>>> I've also cleaned up the handling of profiling data sections a bit. After adding the extra section of data to 
>>>> MethodData, I was seeing several crashes when ciMethodData was used. The underlying issue seemed to be that the 
>>>> offset of the parameter data was computed based on the total data size - parameter data size (which doesn't work if 
>>>> we add an additional section for exception handler data). I've re-written the code around this a bit to try and 
>>>> prevent issues in the future. Both MethodData and ciMethodData now track offsets of parameter data and exception 
>>>> handler data, and the size of the each data section is derived from the offsets.
>>>>
>>>> Finally, there was an assert firing in `freeze_internal` in `continuationFreezeThaw.cpp`:
>>>>
>>>>    assert(monitors_on_stack(current) == ((current->held_monitor_count() - current->jni_monitor_count()) > 0),
>>>>         "Held monitor count and locks on stack invariant: " INT64_FORMAT " JNI: " INT64_FORMAT, (int...
>>>
>>> Jorn Vernee has updated the pull request incrementally with two additional commits since the last revision:
>>>
>>> - add interpreter profiling specific test cases
>>> - rename ex_handler -> exception_handler
>>
>> Another round of tier 1 - 8 testing came back clean. I'm planning to integrate the patch tomorrow.
>>
>> -------------
>>
>> PR Comment: https://git.openjdk.org/jdk/pull/16416#issuecomment-1827516420
>