PING: RFR: JDK-8153074: UL: Show output option in VM.log jcmd
Yasumasa Suenaga
yasuenag at gmail.com
Mon Jun 13 03:45:10 UTC 2016
Hi David,
Thank you for your comment.
> So options are a distinct property of outputs and so should have been a first class entity in LogOutput all along.
I agree to you.
But I think we need to discuss about it with logging folks.
I uploaded a new webrev. It removes fixed buffer length and changes the order of output.
Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.05/
Thanks,
Yasumasa
On 2016/06/13 9:05, David Holmes wrote:
> Hi Yasumasa,
>
> On 13/06/2016 9:30 AM, Yasumasa Suenaga wrote:
>> Hi David,
>>
>> I think "config_string" is different from "option_string".
>>
>> -Xlog format (from -Xlog:help message):
>> -Xlog[:[what][:[output][:[decorators][:output-options]]]]
>>
>> config_string: "what" (ex. gc=trace)
>> option_string: "output-options" (ex. filecount=5)
>>
>> Currently, LogOutput handles tags and loglevels only as config_string.
>> It does not contain output options.
>
> Okay I'm starting to see the bigger picture here. In terms of the overall logging configuration we might have, for example:
>
> gc=trace -> stdout
> runtime=info -> fileA
> compiler=trace -> fileB
>
> where the LHS is (part of) the configuration, and the RHS is the output. So for each output we set its "configuration" to the associated LHS.
>
> So options are a distinct property of outputs and so should have been a first class entity in LogOutput all along.
>
> Okay so looking at your v4 I have two comments:
>
> First, hard-wiring OPTIONS_LEN. I don't know what the possible options are so don't know if 100 is adequate.
>
> Second, if the logging syntax is:
>
> -Xlog[:[what][:[output][:[decorators][:output-options]]]]
>
> then shouldn't the configuration be printed in the same order/format?
>
> Thanks,
> David
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/06/13 8:14, David Holmes wrote:
>>> On 12/06/2016 11:10 PM, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>> Thank you for your comment.
>>>>
>>>>> Is there some reason the option string could not simply become part of
>>>>> the existing configuration string?
>>>>
>>>> My first proposal keeps option string at LogOutput and its child class
>>>> (See webrev.01).
>>>> Marcus commented that option string should be generated from current
>>>> configuration.
>>>>
>>>> I uploaded new webrev.
>>>> Could you review again?
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.04/
>>>
>>> Sorry but I repeat my question - why is the option information not
>>> simply part of the config_string?
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/06/12 6:44, David Holmes wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> Sorry but this API seems poorly fitting to me. First
>>>>> print_option_string seems the wrong name given that the base class,
>>>>> LogOutput, has no notion of having an "option string". It seems to be
>>>>> a supposedly generic "print other stuff" function that only one class
>>>>> actually needs to implement.
>>>>>
>>>>> Secondly it inverts the style of the API used for everything else - we
>>>>> have getters for all the other "properties" which are then printed by
>>>>> the describe_current_configuration method. But this is instead a
>>>>> "print" function where we ask the target to print itself. Mixing the
>>>>> two styles seems messy. It probably would have been better to have had
>>>>> a print-style API from the start - then adding the options would have
>>>>> been a trivial extension for those output classes with options.
>>>>>
>>>>> In addition the change you made to describe_current_configuration is
>>>>> not at all general purpose - you wanted a given format (print between
>>>>> the config string and the decorators) for this one class and so you
>>>>> added the code to support that format. But that format may not make
>>>>> sense for other classes that might have "extra stuff" to print.
>>>>>
>>>>> Is there some reason the option string could not simply become part of
>>>>> the existing configuration string? It seems to me that for a LogFile
>>>>> these "options" really are part of the configuration.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> PS. The two hpp files would need their copyright years updated to
>>>>> "2015, 2016,".
>>>>>
>>>>> On 11/06/2016 10:30 PM, Yasumasa Suenaga wrote:
>>>>>> PING: Could you review it?
>>>>>> We need a second reviewer.
>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.03/
>>>>>>
>>>>>> This change is small fix, and it helps us to confirm current
>>>>>> FileLogOutput configuration.
>>>>>> So I want to merge it to jdk 9.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/05/17 19:17, Yasumasa Suenaga wrote:
>>>>>>> PING: Could you review it?
>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.03/
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2016/05/10 8:06, Yasumasa Suenaga wrote:
>>>>>>>> We need a second reviewer.
>>>>>>>> Could you review it?
>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.03/
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2016/05/04 23:38, Marcus Larsson wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 05/04/2016 04:12 PM, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi Marcus,
>>>>>>>>>>
>>>>>>>>>>> 93 out->print("filecount=%u,filesize=" SIZE_FORMAT "%s ",
>>>>>>>>>>> _file_count, byte_size_in_proper_unit(_rotate_size),
>>>>>>>>>>> proper_unit_for_byte_size(_rotate_size));
>>>>>>>>>>
>>>>>>>>>> Thanks, I applied it to new webrev:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.03/
>>>>>>>>>
>>>>>>>>> Looks OK.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Marcus
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Could you review again?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2016/05/04 22:35, Marcus Larsson wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 05/04/2016 02:59 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>> Hi Marcus,
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for your comment.
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.02/
>>>>>>>>>>>
>>>>>>>>>>> Looks better. The format for _rotate_size should be SIZE_FORMAT.
>>>>>>>>>>>
>>>>>>>>>>> While we're at it I think it would be good (as I mentioned) to
>>>>>>>>>>> use
>>>>>>>>>>> a proper unit for the filesize. Basically changing
>>>>>>>>>>>
>>>>>>>>>>> 93 out->print("filecount=%u,filesize=%lu ", _file_count,
>>>>>>>>>>> _rotate_size);
>>>>>>>>>>>
>>>>>>>>>>> into
>>>>>>>>>>>
>>>>>>>>>>> 93 out->print("filecount=%u,filesize=" SIZE_FORMAT "%s ",
>>>>>>>>>>> _file_count, byte_size_in_proper_unit(_rotate_size),
>>>>>>>>>>> proper_unit_for_byte_size(_rotate_size));
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Marcus
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I fixed to use _rotate_size and _file_count directly to show
>>>>>>>>>>>> VM.log list jcmd.
>>>>>>>>>>>> I do not store option string, and I added new function to print
>>>>>>>>>>>> option string.
>>>>>>>>>>>>
>>>>>>>>>>>> Could you review it again?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 2016/05/04 18:33, Marcus Larsson wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 05/03/2016 01:43 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> PING: Could you review and sponsor it?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.01/
>>>>>>>>>>>>>
>>>>>>>>>>>>> I would prefer to generate the option string from the actual
>>>>>>>>>>>>> options rather than saving the string from when it was
>>>>>>>>>>>>> configured. This would also produce/print the options for
>>>>>>>>>>>>> outputs that are using the defaults (which is not the case
>>>>>>>>>>>>> now).
>>>>>>>>>>>>> The filesize option could then use byte_size_in_proper_unit and
>>>>>>>>>>>>> proper_unit_for_byte_size to make it easier to read.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Also, get_option_string() should just be called
>>>>>>>>>>>>> option_string().
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Marcus
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This patch makes to show option string of LogFileOutput.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2016/04/19 22:55, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>> I adapted changes to jdk9/hs/hotspot repos.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.01/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Please review.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2016/04/18 23:09, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>> PING:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I've sent review request for JDK-8153074.
>>>>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.00/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> If this patch is merged, user can confirm output option via
>>>>>>>>>>>>>>>> VM.log jcmd.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Please review and sponsor it.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2016/04/11 18:29, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>> PING: Could you review and sponsor it?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.00/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 2016/03/31 22:35, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>> CC'ed to serviceability-dev.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.00/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 2016/03/30 23:09, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> This request review is related to [1].
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I want to see output option (filecount, filesize) in
>>>>>>>>>>>>>>>>>>> VM.log jcmd.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Output sample:
>>>>>>>>>>>>>>>>>>> #2: gc.log gc=trace,
>>>>>>>>>>>>>>>>>>> filecount=5,filesize=1048576
>>>>>>>>>>>>>>>>>>> time,level,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I uploaded webrev. Could you review it?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.00/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-March/018704.html
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
More information about the hotspot-runtime-dev
mailing list