[crac] RFR: 8373687: [CRaC] Add metrics through jcmd [v5]

[Timofei Pushkin]

On Wed, 17 Dec 2025 10:20:14 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> Enable user adding custom metrics (and labels) through jcmd JDK.checkpoint command, e.g. using
>> 
>> jcmd <pid> JDK.checkpoint metrics=foo=123,bar=0.456
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix MacOSX

We should introduce a Java API for setting labels too at some point

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

> 712:     // silently ignoring this
> 713:     return true;
> 714:   }

I think we should at least print a warning in such case. I.e. expose a method for the JCMD code to check for support and print the warning to the JCMD stream if there's none. Then here we should fail in such case.

Same for `record_image_score`.

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

> 724:     // silently ignoring this
> 725:     return true;
> 726:   }

Not a part of this PR and we should've discussed this earlier, but why don't we fail here? We provide `is_image_score_supported` for Java to check, so we could make `Score.record()` not call this at all in such case. Same for the user-facing code: `Score.setScore` could throw `UnsupportedOperationException` when `Score.isSupported` returns `false`.

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

> 486: bool CracEngine::set_label(const char* label, const char* value) {
> 487:   return _image_constraints_api->set_label(_conf, label, value);
> 488: }

Nitpick: would be better to group this together with the rest of the `_image_constraints_api` code

src/hotspot/share/services/diagnosticCommand.cpp line 1090:

> 1088: void CheckpointDCmd::execute(DCmdSource source, TRAPS) {
> 1089: 
> 1090:   const char *metrics = _metrics.value();

Suggestion:

void CheckpointDCmd::execute(DCmdSource source, TRAPS) {
  const char *metrics = _metrics.value();

src/hotspot/share/services/diagnosticCommand.cpp line 1092:

> 1090:   const char *metrics = _metrics.value();
> 1091:   if (metrics != nullptr) {
> 1092:     LocaleGuard lg;

What is the problem with locales? Why is this needed for metrics but not labels?

src/hotspot/share/services/diagnosticCommand.cpp line 1171:

> 1169: 
> 1170: #ifdef _WINDOWS
> 1171: static char *strsep(char **strp, const char *delim) {

We have this in two places now, we could move it to `src/hotspot/share/utilities/stringUtils.hpp` for example.

src/hotspot/share/services/diagnosticCommand.cpp line 1250:

> 1248: }
> 1249: 
> 1250: 

Suggestion:

src/hotspot/share/services/diagnosticCommand.hpp line 862:

> 860:   static const char* name() { return "JDK.checkpoint"; }
> 861:   static const char* description() {
> 862:     return "Checkpoint via jdk.crac.checkpointRestore().";

Not caused by to this PR, but `jdk.crac.checkpointRestore()` is a strange reference. It should just say "Initiates a CRaC checkpoint." probably.

test/jdk/jdk/crac/JcmdArgsTest.java line 94:

> 92:     }
> 93: 
> 94:     private static Path createTemp(String metrics, String text) throws IOException {

`metrics` should be `name`

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

PR Review: https://git.openjdk.org/crac/pull/281#pullrequestreview-3604120559
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2639944216
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2639951121
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2639920120
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2640024276
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2640059792
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2640041110
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2640052385
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2639986172
PR Review Comment: https://git.openjdk.org/crac/pull/281#discussion_r2640092774