RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v9]

Sonia Zaldana Calles szaldana at openjdk.org
Wed Sep 11 14:16:08 UTC 2024


On Wed, 11 Sep 2024 13:32:49 GMT, Simon Tooke <stooke at openjdk.org> wrote:

>> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) to Windows.
>> 
>> System.map and System.dump_map are implemented using the Windows API and provide roughly the same information in the same format.  Most of the heavy lifting was implemented by @tstuefe in #16301 - this PR adds the Windows implementation and enables the common code for Windows 64 bit.
>>  
>> [Sample output (with NMT enabled)](https://github.com/user-attachments/files/16663332/vm_memory_map_760.txt)
>
> Simon Tooke has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - prevent VM crash on invalid jcmd
>  - changes from review

Hi Simon, 

Thanks! Looks good. 

Just had a question about the empty filename issue. I don’t think there is a case where a filename parameter should accept an empty string (but I might be wrong). 

If that’s the case, I wonder if it would be better to just deal with all empty filename parameter errors in the dcmd argument parser. 

For example, in [diagnosticArgument.cpp](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/services/diagnosticArgument.cpp#L186
).  Perhaps, we could have a check like this: 


if (strcmp(type(), "FILE") == 0) {
     if (str == NULL || *str == 0) {
       stringStream error_msg;
       error_msg.print("Filename is empty or not specified. %s", str);
       THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), error_msg.base());
     }
     _value = REALLOC_C_HEAP_ARRAY(char, _value, JVM_MAXPATHLEN, mtInternal);
     ...
   }


I haven't tested this but this would probably be better handled in a separate RFE though.

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

PR Comment: https://git.openjdk.org/jdk/pull/20597#issuecomment-2343796609


More information about the hotspot-runtime-dev mailing list