[crac] RFR: 8350088: [CRaC] Stop & restart JFR recording on checkpoint [v4]

[Radim Vansa]

On Tue, 18 Feb 2025 09:31:53 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/crac/Core.java line 35:
>> 
>>> 33: public class Core {
>>> 34:     private static ClaimedFDs claimedFDs;
>>> 35:     private static JfrResource jfrResource = new JfrResource();
>> 
>> Suggestion:
>> 
>>     private static final JfrResource jfrResource = new JfrResource();
>
> Can't this and `JfrResource` itself be placed somewhere in `jdk.jfr` module (internal CRaC packages are already exported there)? E.g. in `jdk.jfr.internal.JVMUpcalls` — I believe it is loaded before `afterRestore` calls start so it has time to register this resource.
> 
> I just think that it would be better to have JFR stuff in JFR module. Also the resource could be simplified (passing `startFlightRecorderAfterRestore` as a runnable won't be needed, I think).

When I start an application without `-XX:StartFlightRecording` and with `-verbose:class` I see that none of the `jdk.jfr` classes are loaded. I think that force-loading it before checkpoint would be a regression.

Also you can't add a resource between checkpoint and restore (even before `afterRestore`) - with `OrderedContext` the registration will succeed but `afterRestore` is not invoked without previous `beforeCheckpoint` call - this is an invariant for `Context` interface.
`BlockingOrderedContext` will get stuck in registration in such situation: this is the default to prevent missing a resource registration, where throwing an exception would not be appropriate as this registration might happen by a concurrent thread along the checkpoint.
The `JfrResource` here works effectively as a Context that does not uphold to the invariant above (`startFlightRecorderAfterRestore` is a registration call).

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

PR Review Comment: https://git.openjdk.org/crac/pull/203#discussion_r1959679037