RFR: 8284686: Interval of < 1 ms disables ExecutionSample events [v6]

[Erik Gahlin]

On Thu, 5 May 2022 13:08:04 GMT, Johannes Bechberger <duke at openjdk.java.net> wrote:

>> test/jdk/jdk/jfr/api/recording/event/TestShortPeriod.java line 63:
>> 
>>> 61:         try (var r = new RecordingStream()) {
>>> 62:             r.enable(EventNames.JVMInformation).withPeriod(Duration.ofNanos(1));
>>> 63:             r.onEvent(e -> r.close());
>> 
>> What happens when the event is, in fact, disabled?
>> The test will run forever, right?
>> 
>> Could you rather use something like `CountDownLatch` which will get decremented in `onEvent()` handler and the test method would wait for that latch for at most N milliseconds - failing when the wait times out?
>
> I could, but lets ask @egahlin for his opinion. He proposed it in the first place.

I think it's better to use the timeout provided by jtreg than hardwiring in a number in the test.

The test could be run on different hardware or in modes that are very slow.  It's better to allow the framework to provide the timeout. The test should not fail, so it shouldn't be an issue.

I think the code looks fine, but could you remove the update to the documentation / specification. Also, perhaps add a comment in the test that the execution event requires a separate thread because the thread streaming events gets excluded from JFR (to avoid recursion).

I will take it for a spin a few hundred times on different platforms to see if there are any intermittent issues.

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

PR: https://git.openjdk.java.net/jdk/pull/8183