[crac] RFR: 8375044: [CRaC] Structured engine options info [v4]

[Timofei Pushkin]

On Wed, 14 Jan 2026 07:26:51 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> Currently the engine documents all exposed options through the description extension; this has some downsides:
>> 
>> * when the JVM wants to change engine option default, -XX:CRaCEngineOptions=help would print wrong value
>> * options completely controlled by JVM (such as image_location) are reported, too
>> * options documentation format is fully up to the engine (and it would be more complicated to reflow lines in the output)
>> 
>> The solution is exposing options info in a more structured manner.
>> 
>> As an extra feature we can use -XX:CRaCEngineOptions=help=foo to limit help to only those containing 'foo' in the name.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix windows build

src/hotspot/share/include/crlib/crlib_description.h line 37:

> 35: 
> 36: // Applicability of the option: setting the value in non-applicable context
> 37: // may result in warnings or errors.

I don't think `DEPRECATED` is about applicability, at least not in the same sense as the other two flags are. Maybe the docs should be separate for each flag?

src/hotspot/share/include/crlib/crlib_description.h line 41:

> 39:   CHECKPOINT = 1 << 0,
> 40:   RESTORE    = 1 << 1,
> 41:   DEPRECATED = 1 << 2,

The values should have globally unique names, like `CRLIB_CONF_OPTION_FLAG_CHECKPOINT`.

E.g. we have `CRLIB_IMAGE_INFO_*` prefix for `crlib_image_info_parts_t` enum in the image info extension of Zulu.

src/hotspot/share/include/crlib/crlib_description.h line 42:

> 40:   RESTORE    = 1 << 1,
> 41:   DEPRECATED = 1 << 2,
> 42: } crlib_conf_option_flag_t;

There may be a problem with ABI compatibility if we'll add more values in the future. Since we do not fix an underlying type (we cannot as this is a C99 API) the compiler is free to choose any integer type not larger than int that can represent all listed values. I suggest adding an upper-bound value that will ensure that either int or unsigned int is the underlying type and only unsigned values are used (on popular data models): `MAX = 0x7FFFFFFF`. The value is not supposed to be actually used.

Although maybe this is too paranoid.

src/hotspot/share/include/crlib/crlib_description.h line 50:

> 48:   const char *value_type;
> 49:   const char *default_value;
> 50:   const char *description;

I'd prefer to have nullability specified for the strings. I think all but `default_value` must be non-nullable. `default_value` probably should be nullable with null representing options without which the engine will always fail C/R (e.g. image and engine locations in case of crexec).

src/hotspot/share/include/crlib/crlib_description.h line 72:

> 70:   // Some keys can be excluded if these are not supposed to be set by a user but rather by the
> 71:   // application the engine is linked to, or if these are deprecated.
> 72:   //

For compatibility we shouldn't change existing API like this. But we can add a deprecation note for the method suggesting switching to the new `configuration_options` method.

src/hotspot/share/include/crlib/crlib_description.h line 89:

> 87:   // Returns an array of all configuration options supported by the engine.
> 88:   // The array is terminated with an option with null key.
> 89:   // Set to null if this method is not supported.

We decided against allowing nulls for unsupported methods. Instead, the app should not use methods that are defined beyond `extension->header.size`, and the engine should return an error value (null in this case) when the method exists in the API, is called, but the engine does not support it.

src/hotspot/share/runtime/crac.cpp line 553:

> 551:   } else {
> 552:     if (pattern != nullptr) {
> 553:       log_warning(crac)("Option filtering by pattern not available");

Would be cleaner to print `Configuration options:` instead of `Configuration options matching *%s*:` — just a matter of moving the printing under the `options != nullptr` check

src/hotspot/share/runtime/crac_engine.cpp line 409:

> 407:   require_method(configurable_keys)
> 408:   require_method(supported_extensions)
> 409:   // configuration_options is not mandatory

We should rely on `_description_api->header.size` instead

src/hotspot/share/runtime/crac_engine.cpp line 424:

> 422: 
> 423: const crlib_conf_option_t *CracEngine::configuration_options() const {
> 424:   static crlib_conf_option_t *all_options = nullptr;

I doubt that the documentation code will be performance-critical so I'd prefer to remove this caching to leave the possibility of using `CracEngine` for different engines within a process. I don't have a practical use case yet, just seems cleaner.

src/hotspot/share/runtime/crac_engine.cpp line 440:

> 438:   crlib_conf_option_t *dst = all_options;
> 439:   for (src = options; src->key != nullptr; ++src, ++dst) {
> 440:     memcpy(dst, src, sizeof(*src));

Better to copy after the skip check

src/java.base/share/native/libcrexec/crexec.cpp line 203:

> 201: #undef ADD_ARR_ELEM
> 202: #define ADD_ARR_ELEM(id, ctype, cdef, flags, ...) { opt_##id, static_cast<crlib_conf_option_flag_t>(flags), __VA_ARGS__ },
> 203: static const crlib_conf_option_t configure_options[] = { CONFIGURE_OPTIONS(ADD_ARR_ELEM) {} };

Can't the arrays still be constexpr?

src/java.base/share/native/libcrexec/crexec.cpp line 248:

> 246:   };
> 247: 
> 248: // Note: image_location, exec_location and args from this are ignored

Maybe split `CONFIGURE_OPTIONS` onto `CHECKED_OPTIONS` (w/ a field) and `UNCHECKED_OPTIONS` (w/o a field)? Otherwise the majority of the fields created here are redundant.

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

PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703871002
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703856219
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703846454
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703930933
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703942271
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2703960918
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704013689
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704044221
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704055572
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704076415
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704110480
PR Review Comment: https://git.openjdk.org/crac/pull/286#discussion_r2704128290