On Thu, 13 Nov 2025 19:41:40 GMT, Mat Carter <[email protected]> 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
