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

[Timofei Pushkin]

On Tue, 18 Feb 2025 08:55:52 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix module info
>
> src/hotspot/share/jfr/jni/jfrUpcalls.cpp line 230:
> 
>> 228:     CLEAR_PENDING_EXCEPTION;
>> 229:     ResourceMark rm(THREAD);
>> 230:     log_error(jfr, system)("JfrUpcall failed for %s", unhide_internal_types_sym->as_C_string());
> 
> Can't we propagate the exception? If not, the method should probably not be marked as `TRAPS` and the `CHECK` above should be transformed to this exception clearing as well?

Also, `unhide_internal_types_sym` should be replaced with `request_start_after_restore_sym`

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

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

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