On Thu, 12 Nov 2026 09:31:28 GMT, Alan Bateman <[email protected]> wrote:
>> Could I have a review of a PR that fixes several issues with the JFR >> exception events: >> >> - Error and its subclasses emitted two JavaExceptionThrow events >> - Error was counted twice in ExceptionStatistics >> - Constructors that only take the cause lost the message >> >> >> The testing was inadequate, so I rewrote most of it. >> >> Testing: jdk_lang, jdk_jfr >> >> Thanks >> Erik >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > test/jdk/jdk/jfr/event/runtime/TestExceptionEvents.java line 41: > >> 39: /* >> 40: * @test >> 41: * @bug 8397069 > > I assume this should list 8199712 or whatever JBS issue motivated the test > originally so that it doesn't look like 8397069 is the motivation for the > test. I will drop the bug number. It's not clear what it should be, and the Git history is a better source of truth. > test/jdk/jdk/jfr/event/runtime/TestExceptionEvents.java line 47: > >> 45: * @run main/othervm jdk.jfr.event.runtime.TestExceptionEvents >> 46: */ >> 47: @SuppressWarnings("serial") > > Is this because serialVersionUID has been removed from the nested > classes/exceptions? Yes, I thought this was a more appropriate way to avoid the warnings than having multiple serialVersionUID fields that never will be used. > test/jdk/jdk/jfr/event/runtime/TestExceptionEvents.java line 83: > >> 81: try (Recording r = new Recording()) { >> 82: r.enable(ERROR_EVENT_NAME).withStackTrace(); >> 83: r.enable(EXCEPTION_STATISTICS_NAME).with("period", >> "everyChunk"); > > I'm hazy on the "everyChunk" period so I think it means beginning and end of > each chunk. Does this have any interaction with higher tiers run that enable > JFR? Different recording of course but I wondering chunk rotation could cause > interference. By default events become "everyChunk", which (today) means they will be emitted at the beginning and the end of the chunk. I can make et explicit and add a comment since the test relies on it. The event rate is too low to trigger a chunk rotation. The file size is about 0.5 MB. 12 MB is needed to rotate. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33828#discussion_r4549213170 PR Review Comment: https://git.openjdk.org/jdk/pull/33828#discussion_r4549232797 PR Review Comment: https://git.openjdk.org/jdk/pull/33828#discussion_r4549251532
