On Sat, 16 Mar 2024 12:50:05 GMT, Matthias Baesken <mbaes...@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
