RFR: 8370203 - Add jcmd AOT.end_recording diagnostic command [v3]

[Vladimir Kozlov]

On Thu, 13 Nov 2025 19:41:40 GMT, Mat Carter <macarte at openjdk.org> wrote:

>> Add jcmd AOT.end_recording diagnostic command.  When this command is issued, a targeted JVM that is currently recording AOT information will stop recording.  Existing functionality is preserved: when stopped the JVM will create the required artifacts based on the execution mode.  Conveniently as the application running on the JVM has not stopped (as was previously the only way to stop recording), the application will resume execution after the artifacts have been generated.
>> 
>> The command will report back to the user one of the following messages depending on the state of the JVM:
>> 
>> - Error! Not a recording run
>> - Error! Not recording
>> - Recording ended successfully
>> - Error! Failed to end recording
>> 
>> It follows that issues the command to a JVM that is recording, twice in succession, should (baring internal errors) would produce the following two responses:
>> 
>> - Recording ended successfully
>> - Error! Not recording
>> 
>> Passes tier1 on linux (x64) and windows (x64)
>
> Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Document use of DiagnosticCommand

Comments.

src/hotspot/share/cds/aotMetaspace.cpp line 961:

> 959: }
> 960: 
> 961: bool AOTMetaspace::is_recording_preimage_static_archive() {

I think the name should be `preimage_static_archive_dumped()` to match field's name you accessing and check ` == 1` instead. The code in `AOTEndRecordingDCmd::execute()` will be more clear then.

src/hotspot/share/cds/aotMetaspace.cpp line 962:

> 960: 
> 961: bool AOTMetaspace::is_recording_preimage_static_archive() {
> 962:   if (CDSConfig::is_dumping_preimage_static_archive()) {

This is called after you already checked `CDSConfig::is_dumping_preimage_static_archive()`. May be replace the check with assert.

src/hotspot/share/cds/aotMetaspace.cpp line 973:

> 971:       return;
> 972:     }
> 973:   }

Add comment that this could be called from different threads and possibly concurrently: when JCMD requested it and normal call during VM exit. We should dump archive only once.

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

> 994:     output()->print_cr("Error! Not a recording run");
> 995:     return;
> 996:   }

First, is `output()` directed to VM's tty or somewhere else?

Second, I think you need to be more verbose in error. Maybe:

Error: AOT recording is not initiated. VM flags -XX:AOTMode=record or -XX:AOTCacheOutput=<file> are missing.

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

> 998:   if (!AOTMetaspace::is_recording_preimage_static_archive()) {
> 999:     output()->print_cr("Error! Not recording");
> 1000:     return;

This check is confusing because your error message is cryptic.
Actually I think you don't need any message here - it is not error. Use comment instead:

  // The other thread already initiated end of recording.

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

> 1005:     output()->print_cr("Recording ended successfully");
> 1006:     return;
> 1007:   }

I think this should be reversed and message replaced with comment.

if (AOTMetaspace::is_recording_preimage_static_archive()) {
    // Other thread already dumped archive.
    return;
}
output()->print_cr("AOT recording ended successfully");

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

PR Review: https://git.openjdk.org/jdk/pull/27965#pullrequestreview-3466124096
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528503476
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528453164
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528486742
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528446012
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528465285
PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2528496380