RFR: 8137164: Include PID in the JFR jcmd hint

[Erik Gahlin]

Hi Thomas,

> On 28 Jun 2018, at 07:38, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi Erik,
> 
> this looks fine to me.
> 
> Since you are changing the output, could we change filename=FILEPATH
> to filename=<FILEPATH> or filename=<filepath>, which seems to be the
> default for "insert name here”?

I don’t remember the rationale for FILEPATH. I will look into it. 

Perhaps it was because a shorter placeholder is more likely to fit 80 character.  That was at least the reason the PID was not included originally, but I think the usability aspects of having it printed outweighs such concerns.

It could also be because of how jcmd <pid> help JFR.start references parameters in the help text.

Regardless, I would prefer to handle that as a separate issue.

> 
> Side note, I was looking at this code closely for the first time and I
> am surprised how much parsing etc is done on the client side (the
> jcmd) -

If you mean the changes in the review, it all happens server side.

Thanks
Erik

> I always thought the principle was to leave jcmd dumb and thus
> compatible and let the hotspot itself do all the work? Sorry if the
> question is stupid, I do not know much about JFR.
> 
> Thanks, Thomas
> 
> 
> On Wed, Jun 27, 2018 at 10:09 PM, Erik Gahlin <erik.gahlin at oracle.com> wrote:
>> Hi,
>> 
>> Could I have review of the following fix that adds the PID to the output
>> when starting a JFR recording.
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~egahlin/8137164/
>> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8137164
>> 
>> Testing:
>> test/jdk/jdk/jfr/*
>> 
>> Thanks
>> Erik