PING: RFR: JDK-8153074: UL: Show output option in VM.log jcmd
Yasumasa Suenaga
yasuenag at gmail.com
Sun Jun 12 23:30:31 UTC 2016
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.
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