RFR: 8319374: JFR: Remove instrumentation for exception events

[Erik Gahlin]

On Mon, 6 Nov 2023 15:49:02 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Could I have a review of a PR that removes the bytecode instrumentation for the exception events.
>> 
>> Testing: jdk/jdk/jfr + tier1 + tier2
>
> src/java.base/share/classes/jdk/internal/event/ThrowableTracer.java line 37:
> 
>> 35:     private static final AtomicLong numThrowables = new AtomicLong();
>> 36: 
>> 37:     public static void enable() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
> 
> SecurityException and IllegaArgumentException are unchecked so don't need them in the throws declaration.

Will fix.

> src/java.base/share/classes/jdk/internal/event/ThrowableTracer.java line 44:
> 
>> 42: 
>> 43:     public static void traceError(Class<?> clazz, String message) {
>> 44:         if (OutOfMemoryError.class.isAssignableFrom(clazz)) {
> 
> StackOverflowError is likely problematic too, maybe it should be VirtualMachineError.

I remember asking the same question ten years ago, when Nils did the original implementation, but I think it was only needed for OOM, because it creates an infinite loop when the event object was allocated, which resulted in a StackOverflowError instead OOM.

Some OOM tests failed with JFR enabled.

The event object allocation has been removed, but I think we can run into the allocation recursion by other means. I looked into it a few years ago, but I don't remember exactly why it failed.

If a SOE happens, I think we are fine. There is something that prevents infinite recursion when the SOE object is created. Perhaps it is preallocated? I prefer to not change the behavior, at least not in this PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16493#discussion_r1384124648
PR Review Comment: https://git.openjdk.org/jdk/pull/16493#discussion_r1384124213