On Tue, 23 Jul 2024 17:57:04 GMT, Sonia Zaldana Calles <szald...@openjdk.org> wrote:
>> Hi all, >> >> This PR addresses [8334492](https://bugs.openjdk.org/browse/JDK-8334492) >> enabling jcmd diagnostic commands that issue an output file to accept the >> `%p` pattern in the file name and substitute it for the PID. >> >> This PR addresses the following diagnostic commands: >> - [x] Compiler.perfmap >> - [x] GC.heap_dump >> - [x] System.dump_map >> - [x] Thread.dump_to_file >> - [x] VM.cds >> >> Note that some jcmd diagnostic commands already enable this functionality >> (`JFR.configure, JFR.dump, JFR.start and JFR.stop`). >> >> I propose opening a separate issue to track updating the man page similarly >> to how it’s done for the JFR diagnostic commands. For example, >> >> >> filename (Optional) Name of the file to which the flight recording >> data is >> written when the recording is stopped. If no filename is >> given, a >> filename is generated from the PID and the current date >> and is >> placed in the directory where the process was started. The >> filename may also be a directory in which case, the >> filename is >> generated from the PID and the current date in the >> specified >> directory. (STRING, no default value) >> >> Note: If a filename is given, '%p' in the filename will be >> replaced by the PID, and '%t' will be replaced by the >> time in >> 'yyyy_MM_dd_HH_mm_ss' format. >> >> >> Unfortunately, per [8276265](https://bugs.openjdk.org/browse/JDK-8276265), >> sources for the jcmd manpage remain in Oracle internal repos so this PR >> can’t address that. >> >> Testing: >> >> - [x] Added test case passes. >> - [x] Modified existing VM.cds tests to also check for `%p` filenames. >> >> Looking forward to your comments and addressing any diagnostic commands I >> might have missed (if any). >> >> Cheers, >> Sonia > > Sonia Zaldana Calles has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 15 commits: > > - Merge master > - Fixing formatting > - Inlining buffer and making field private > - Reverting to functional changes in parserTests.cpp > - Error messaging format > - Fixing memory leak > - Fixing pointer style, s/NULL/nullptr, and exception > - Cleaning up parserTests.cpp > - Missing copyright header update > - Adding tests for file dcmd argument > - ... and 5 more: https://git.openjdk.org/jdk/compare/2f2223d7...52ca557d Is the help output working OK? Do these commands' help outputs show the new %p filename? I think it's good that they would. We should expect users of these commands to implicity understand a %p although we can still explain it, e.g. in a separate update in the man page. I just think we should be explicit that the help output is changing. In src/hotspot/share/runtime/java.cpp: if (DumpPerfMapAtExit) { CodeCache::write_perf_map(.... It may need to pass DEFAULT_PERFMAP_FILENAME (and tty). Do you have the change from JDK-8327054 merged into this branch? src/hotspot/share/services/diagnosticArgument.hpp line 65: > 63: class FileArgument { > 64: private: > 65: char _name[1024]; Probably JVM_MAXPATHLEN (which might also be 1024). ------------- PR Comment: https://git.openjdk.org/jdk/pull/20198#issuecomment-2247558224 PR Comment: https://git.openjdk.org/jdk/pull/20198#issuecomment-2247560119 PR Review Comment: https://git.openjdk.org/jdk/pull/20198#discussion_r1689545224
