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

Thu Nov 30 19:14:37 UTC 2023

Hello,

It seems to be an issue with XCode 12.2 not supporting the [[noreturn]] 
attribute. Note that the build guide recommends at least XCode 14 [1], 
so you may want to upgrade XCode to see if that helps.

Jorn

[1]: https://github.com/openjdk/jdk/blob/master/doc/building.md#macos

On 30/11/2023 19:48, Vladimir Kozlov wrote:
> 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
>>