RFR: 8331876: JFR: Move file read and write events to java.base [v3]

[Erik Gahlin]

On Sat, 11 May 2024 15:00:09 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> A compromise between performance and readability is:
>> 
>> if (JFRTracing.isEnabled()) {
>>       ...
>> }
>> 
>> One additional class is loaded, but it's more clear where it comes from. I didn't want to do that for the ThrowableTracer class since it had a clinit.
>> 
>> This could potentially cause problems if JFRTracing is loaded early from Throwable or other class in the future. The static boolean flag is more safe, so probably better.
>
> One thing that isn't clear (to me anyway) is how it works with the memory model. It's plain read at the use sites, looks like the set when recording is turned on is also a plain write. Would it be possible to summarise what else happens when recording is turned on?

During JFR initialization, jfrTracing flag is set to true by the JFRTracing class. Then, when the recording starts, a safepoint occurs, and all Java threads are stopped at a poll site. When they wake up and eventually read the memory of the jfrTracing field, it will be true because of memory fences related to the safepoint.

I guess it's not 100% safe if the JIT decides to store the read value elsewhere over several event checks, but it seems unlikely. Event settings checks (i.e., Event::isEnabled()) have always used plain reads, so it is not more unreliable than any other event.

I'm fine with using a volatile too. I used it for the exception events, but I now think a plain write/read of jfrTracing is safe for all practical purposes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19129#discussion_r1597639174