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

Timofei Pushkin tpushkin at openjdk.org
Tue Feb 18 18:54:13 UTC 2025


On Tue, 18 Feb 2025 12:45:20 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> 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).

I was thinking about registering between `beforeCheckpoint` and `afterRestore` calls, yes. It would be strange but I didn't find what part of the code exactly will prevent us by doing this. But it's probably better to leave this as is, I agree.

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

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


More information about the crac-dev mailing list