[crac] RFR: 8364662: [CRaC] Process restore options in the common arguments parser

Timofei Pushkin tpushkin at openjdk.org
Thu Aug 7 09:02:37 UTC 2025


On Thu, 7 Aug 2025 08:01:45 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> Options for restore are now processed during the common VM arguments parsing stage.
>> 
>> Notable improvements compared to the old code:
>> - Options from `-XX:Flags=<file>` and `-XX:VMOptionsFile=<file>` are now also considered
>> - Option aliasing is now handled correctly
>> 
>> **Behavioral change**. If `CRaCIgnoreRestoreIfUnavailable` is true it is now not an error to specify non-restore-settable options on restore — these will just be omitted from forwarding to the restored JVM. This makes it possible to specify non-restore-settable to be used after a failed restore.
>
> src/hotspot/share/runtime/arguments.cpp line 1046:
> 
>> 1044: }
>> 1045: 
>> 1046: void Arguments::build_jvm_restore_flags(const char* arg) {
> 
> I would use `append_jvm_restore_flags` - `build` implies the final operation on a builder.

I follow the style of the class: there are `Arguments::build_jvm_args` and `Arguments::build_jvm_flags` which do exactly the same thing, just for two different arrays.

> src/hotspot/share/runtime/crac.cpp line 638:
> 
>> 636:         // should be concatenated. But with the current code the last occurence
>> 637:         // will just overwrite the previous ones.
>> 638:         assert(!JVMFlag::find_flag(cursor)->ccstr_accumulates(),
> 
> Ouch! I think that using multiple `-XX:CRaCEngineOptions` would be pretty common (at least some of my scripts do that). Could we fix this?
> 
> And this error should be guarantee/another check that is applied in release build, too because it verifies user input, not some invariants in internal components.

This is not an issue for `CRaCEngineOptions` because it is not applied in the restored process (you can see it is handled in another branch above).

Currently there are no `ccstrlist` restore-settables actually being applied in the restored process, that is why I just added this assert for now. Its purpose is to fail when we add such option.

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

PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259626820
PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259620693


More information about the crac-dev mailing list