On Thu, 20 Nov 2025 16:22:29 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: >> >> - AOT.end_recording is unsupported when VM flags -XX:AOTMode=record or >> -XX:AOTCacheOutput=<file> are missing >> - Recording has already ended. >> - Error: Failed to end recording. >> - Recording ended successfully. >> >> 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. >> - Recording has already ended. >> >> Passes tier1 on linux (x64) and windows (x64) > > Mat Carter has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge branch 'JDK-8370203' of https://github.com/macarte/jdk into > JDK-8370203 > - Corrected placement of assert and logic test Changes requested by lmesnik (Reviewer). src/hotspot/share/cds/aotMetaspace.cpp line 1062: > 1060: bool AOTMetaspace::preimage_static_archive_dumped() { > 1061: assert(CDSConfig::is_dumping_preimage_static_archive(), "Required"); > 1062: return _preimage_static_archive_dumped == 1; Should it be AtomicAccess::load(&_preimage_static_archive_dumped) here? test/hotspot/jtreg/runtime/cds/appcds/aotCache/DiagnosticCommandMBeanTest.java line 98: > 96: out.shouldContain("Failed to stop recording"); > 97: } > 98: out.shouldNotContain("IOException occurred!"); The 'IOException occurred!' is never printed. While there are a lot of other exception print and not checked. Also makes sense to check exit status. test/hotspot/jtreg/runtime/cds/appcds/aotCache/JcmdAOTEndRecordingTest.java line 31: > 29: * @summary Sanity test for Jcmd AOT.end_recording command > 30: * @library /test/lib > 31: * @build JcmdAOTEndRecordingTest No need to build test explicitly, jtreg does it by itself. test/hotspot/jtreg/runtime/cds/appcds/aotCache/JcmdAOTEndRecordingTest.java line 57: > 55: try { > 56: OutputAnalyzer output = > ProcessTools.executeProcess(jcmd.getCommand()); > 57: output.shouldContain("AOT.end_recording is unsupported"); A little bit confused. The check doesn't match test description. ------------- PR Review: https://git.openjdk.org/jdk/pull/27965#pullrequestreview-3490649441 PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2548182449 PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2548168094 PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2548140140 PR Review Comment: https://git.openjdk.org/jdk/pull/27965#discussion_r2548148536
