RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]

[Matthias Baesken]

On Sat, 16 Mar 2024 12:50:05 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> src/hotspot/share/services/diagnosticCommand.cpp line 475:
>> 
>>> 473: HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
>>> 474:                            DCmdWithParser(output, heap),
>>> 475:   _filename("filename","Name of the dump file", "STRING", false, "if no filename was specified, but -XX:HeapDumpPath=hdp is set, path hdp is taken"),
>> 
>> This seems clumsy, but I'm having a hard time coming up with something better.
>> 
>> "the filename specified by -XX:HeapDumpPath, if specified"
>> "If -XX:HeapDumpPath is specified, then it is used as the default"
>> 
>> Makes me wonder if this approach is wrong since it is hard to document clearly. Maybe we should go back to not having the jcmd use HeapDumpPath. I understand the argument for having the jcmd use the HeapDumpPath setting, but it might not be worth the documentation confusion it introduces. You can argue that HeapDumpPath really is just intended to help support HeapDumpOnOutOfMemoryError. Does the jcmd also use HeapDumpGzipLevel? I'm not sure, but we should be consistent with the application of HeapDumpPath and HeapDumpGzipLevel to the jcmd.
>
> So far we only use the gzip level setting from jcmd, not HeapDumpGzipLevel .
> See the `level` variable in  `HeapDumpDCmd::execute` . Yeah you are probably right we should make it consistent.

> If -XX:HeapDumpPath is specified, then it is used as the default

No, the filename set with jcmd GC:heamp_dump  filename   has priority. So we should better keep the current description.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1530077879