On Mon, 15 Jan 2024 07:32:31 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This check is problematic:
>>
>>
>> if (strncmp(filename, DEFAULT_PERFMAP_FILENAME,
>> strlen(DEFAULT_PERFMAP_FILENAME)) == 0)
>>
>>
>> Because it cannot tell whether `filename` was explicitly given by the user.
>> As a result, if the user specifies:
>>
>>
>> jcmd 1234 perfmap '/tmp/perf-<pid>.map'
>>
>>
>> the file will be written as `/tmp/perf-1234.map`, but if the user specifies
>>
>>
>> jcmd 1234 perfmap '/tmp/<pid>-perf.map'
>>
>>
>> then the file will be written as `/tmp/<pid>-perf.map`.
>>
>> This gives the confusing impression that the string `<pid>` is sometimes
>> substituted and sometimes not.
>>
>> I think it's better to do this (similarly for the `VM.cds` case):
>>
>>
>> void PerfMapDCmd::execute(DCmdSource source, TRAPS) {
>> CodeCache::write_perf_map(_filename.is_set() ? _filename.value() :
>> nullptr);
>> }
>>
>>
>> This essentially makes the JVM behavior exactly the same as before, so I
>> think we can change the JBS issue title to something like "Clarify docs for
>> filename parameter to Compiler.perfmap and VM.cds".
>
> @iklam but IIUC that does not address the issue of updationg the help output.
My suggestion is to be used in combination with this change:
_filename("filename", "Name of the map file", "STRING", false,
DEFAULT_PERFMAP_FILENAME)
So the help message will be printed as:
*filename*: (Optional) The name of the map file (STRING /tmp/perf-<pid>.map)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17359#discussion_r1452040385
