RFR: JDK-8177763: Getting an hprof dump via jcmd could benefit from stronger option checking

[Gary Adams]

Here are the items I'd like to discuss.

    1. The current help doc only mention <filename>
        as a positional argument.
        The original bug was posted with an expectation
         that <filename=value> should be accepted.
        Do we document that we support either form
        in the help message?
        Do we change the issue type from bug to rfe?
        Do we need a CSR if the dcmd syntax is changed?

   2. Do we have a native function that will declare a
       path as absolute or relative to the current directory?
       Worried a little about windows paths here.

   3. Looks like minimal heap_dump testing here:
       open/test/hotspot/jtreg/serviceability/dcmd/gc/HeapDumpTest.java
...

On 8/29/18, 2:04 PM, Chris Plummer wrote:
> Hi Gary,
>
> Ok. Overall looks good. A couple of comments:
>
> You now output the path of the created file, but in a kind of strange 
> way. You print the CWD, and then the specified path, whether relative 
> or full. So it's kind of confusing because if the user specifies a 
> full path, they'll still see the CWD printed out. Why not just print 
> the full path (after combining with CWD if needed), or maybe omit the 
> CWD off when a full path was specified. If determining if CWD ends up 
> getting used is difficult, maybe just make it more clear what you are 
> printing is the CWD, and it is not necessarily part of the actual 
> filename path.
>
> What about writing some tests cases. It would probably be pretty easy 
> to update some of the existing test cases to use key=<value> where 
> previously they just allowed <value>.
>
> thanks,
>
> Chris
>
>
> On 8/29/18 8:10 AM, Gary Adams wrote:
>> Sure no problem.
>>
>> I have two workspaces I use called jdk-jdb and jdk-jdk.
>> I have the changes in the jdk-jdb workspace for the heap
>> filename updates.
>>
>> In the jdk-jdb workspace a launch a process that just waits for input.
>>
>>    jdk/bin/java -cp ~/classes my
>>
>> In the jdk-jdk workspace I issue the jcmd to talk to the java process
>> running in the jdk-jdb workspace. The updated output includes
>> "[<current-directory>,<heap dump filename>]"
>>
>> ...
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump filename=foo
>> 2328:
>> Heap dump file created 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,foo]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump filename
>> 2328:
>> Heap dump file created 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,filename]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump ffoo2
>> 2328:
>> Heap dump file created 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,ffoo2]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump ffoo2
>> 2328:
>> File exists [/export/users/gradams/ws/jdk-jdb/build/linux-x64,ffoo2]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump ~/ffoo2
>> 2328:
>> Heap dump file created 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,/home/gradams/ffoo2]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump ~/ffoo2
>> 2328:
>> File exists 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,/home/gradams/ffoo2]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump /tmp/ffoo2
>> 2328:
>> Heap dump file created 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,/tmp/ffoo2]
>>
>> $  jdk/bin/jcmd 2328 GC.heap_dump /tmp/ffoo2
>> 2328:
>> File exists 
>> [/export/users/gradams/ws/jdk-jdb/build/linux-x64,/tmp/ffoo2]
>>
>> $ pwd
>> /home/gradams/scratch/ws/jdk-jdk/build/linux-x64
>>
>>
>> On 8/29/18, 2:21 AM, Chris Plummer wrote:
>>> Hi Gary,
>>>
>>> What would be really useful are some before/after examples that 
>>> demonstrate what has changed from the users point of view.
>>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 8/28/18 5:26 AM, Gary Adams wrote:
>>>> This message may have been lost in the vacation email backlog, so I'm
>>>> sending it again.
>>>>
>>>> On 8/9/18, 2:19 PM, Gary Adams wrote:
>>>>> Thee are several reported problems using jcmd for obtaining heap 
>>>>> dumps.
>>>>>
>>>>> Some users are confused by the positional argument for a filename
>>>>> when a similar jfr command uses "key=value" syntax.
>>>>>
>>>>> Some users are confused by the basic nature of the command, where 
>>>>> the jvm
>>>>> executing the heap dump is running in a different current 
>>>>> directory than
>>>>> the jcmd itself.
>>>>>
>>>>> This is a proposed fix to report the current directory and the 
>>>>> filename
>>>>> when the heap dump is successfully written or if an error occurs. 
>>>>> There
>>>>> is also a proposed fix to handle the case when the user provided
>>>>> "key=value" inputs. If the key matches the argument name, then the
>>>>> user intended the value to be used.
>>>>>
>>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8177763
>>>>>   Webrev: http://cr.openjdk.java.net/~gadams/8177763/webrev.00/
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>