[crac] RFR: 8367975: [CRaC] Pattern in CRaCCheckpointTo

[Radim Vansa]

On Tue, 23 Sep 2025 09:09:23 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> Add support for pattern in `CRaCCheckpointTo` VM option.
>
> src/hotspot/share/runtime/crac.cpp line 294:
> 
>> 292:   if (!_engine->configure_image_location(image_location)) {
>> 293:     return JVM_CHECKPOINT_ERROR;
>> 294:   }
> 
> Is there a reason we do this and CPU features recording below so late instead of at the beginning `crac::checkpoint`, for example? The only reason I see is that the timestamps in `crac::record_time_before_checkpoint` and `%t` would be further away in that case (BTW, if we leave this as is we could use the recorded time for the pattern for consistency).
> 
> If we did not clear the recorded user data on checkpoint, CPU features could be recorded just once on VM initialization instead of on every checkpoint (though not in `prepare_checkpoint` — that is too early, I believe).
> 
> And just to have it more compact since the calls are very closely related
> Suggestion:
> 
>   // Note that CRaCEngine and CRaCEngineOptions are not updated (as documented)
>   // so we don't need to re-init the whole engine handle.
>   if (!resolve_image_location(image_location, sizeof(image_location), &ignored) ||
>       !ensure_image_location(image_location, false) ||
>       !_engine->configure_image_location(image_location)) {
>     return JVM_CHECKPOINT_ERROR;
> }

In this PR I have merely reused the place; I don't see much incentive to move it earlier on. I think this is up to preference between
1) doing some (reversible) work on checkpoint and then (possibly) failing due to no access to the directory, vs. 
2) creating the directory, (possibly) failing during checkpoint and leaving the filesystem tainted (or implementing cleanup)
I don't have a strong preference, but I agree that we can collapse the two `if`s.

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

PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2372213659