Thanks David! I'm waiting to approve FC extension request. Marcus, David, could you be a sponsor for this?
Yasumasa 2016/07/01 11:22 "David Holmes" <david.hol...@oracle.com>: > On 30/06/2016 11:13 PM, Yasumasa Suenaga wrote: > >> Hi Marcus, >> >> Looks good, thanks for fixing this. >>> >> >> Thanks! >> I'm waiting a second reviewer and accepting FC extension request. >> > > Reviewed. This looks much neater. Thanks. > > David > ----- > > > >> Yasumasa >> >> >> On 2016/06/30 22:10, Marcus Larsson wrote: >> >>> Hi >>> >>> >>> On 2016-06-30 15:01, Yasumasa Suenaga wrote: >>> >>>> Hi Marcus, >>>> >>>> Can we keep the printing of the index # in LogConfiguration? That >>>>> would save us from passing it as a parameter to describe. (So, print >>>>> index, call describe, and then print newline.) >>>>> >>>> >>>> I've fixed it. >>>> Could you review again? >>>> >>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.07/ >>>> >>> >>> Looks good, thanks for fixing this. >>> >>> Marcus >>> >>> >>>> >>>> Thanks, >>>> >>>> Yasumasa >>>> >>>> >>>> On 2016/06/30 18:38, Marcus Larsson wrote: >>>> >>>>> Hi, >>>>> >>>>> >>>>> On 2016-06-30 11:31, Yasumasa Suenaga wrote: >>>>> >>>>>> Hi Marcus, >>>>>> >>>>>> Therefore I suggest that we introduce a describe() function in >>>>>>>> LogOutput as part of this change, and move the code currently in >>>>>>>> LogConfiguration::describe to this function, adding the option >>>>>>>> text to it as well. >>>>>>>> >>>>>>> >>>>>> Ah, I understood. >>>>>> If we refactor that in this enhancement, we do not need to make >>>>>> dynamic memory allocation. >>>>>> >>>>>> I uploaded a new webrev. >>>>>> I hope this webrev matches your suggestion :-) >>>>>> >>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.06/ >>>>>> >>>>> >>>>> Looks good! Just a nit: Can we keep the printing of the index # in >>>>> LogConfiguration? That would save us from passing it as a parameter >>>>> to describe. (So, print index, call describe, and then print newline.) >>>>> >>>>> Thanks, >>>>> Marcus >>>>> >>>>> >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Yasumasa >>>>>> >>>>>> >>>>>> On 2016/06/28 22:21, Yasumasa Suenaga wrote: >>>>>> >>>>>>> Hi Marcus, >>>>>>> >>>>>>> I don't really like that we need to make dynamic allocations here. >>>>>>>> >>>>>>> >>>>>>> Should use resource area? or char array? >>>>>>> If we should use char array, how long should we reserve for buffer? >>>>>>> >>>>>>> >>>>>>> Therefore I suggest that we introduce a describe() function in >>>>>>>> LogOutput as part of this change, and move the code currently in >>>>>>>> LogConfiguration::describe to this function, adding the option >>>>>>>> text to it as well. >>>>>>>> >>>>>>> >>>>>>> I think this is refactoring of LogOutput and LogConfiguration. >>>>>>> Now (after FC date), is this work accepted? >>>>>>> >>>>>>> IMHO, refactoring is another enhancement from this. >>>>>>> If it is needed, I think this enhancement should be started after >>>>>>> refactoring. >>>>>>> >>>>>>> >>>>>>> If refactoring and this enhancement can be merged and be accepted, >>>>>>> I will start to work for it. >>>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> Yasumasa >>>>>>> >>>>>>> >>>>>>> On 2016/06/28 20:23, Marcus Larsson wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> >>>>>>>> On 06/28/2016 11:29 AM, Yasumasa Suenaga wrote: >>>>>>>> >>>>>>>>> PING: Could you review and sponsor it? >>>>>>>>> >>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153074/webrev.05/ >>>>>>>>>>> >>>>>>>>>> >>>>>>>> I don't really like that we need to make dynamic allocations >>>>>>>> here. I would prefer to have the outputs be responsible for >>>>>>>> describing themselves just like David mentions. The current >>>>>>>> design of LogConfiguration::describe doesn't follow that pattern, >>>>>>>> but I really think it should. Therefore I suggest that we >>>>>>>> introduce a describe() function in LogOutput as part of this >>>>>>>> change, and move the code currently in LogConfiguration::describe >>>>>>>> to this function, adding the option text to it as well. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Marcus >>>>>>>> >>>>>>>> >>>>>>>>> I've requested FC extension for this. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> Yasumasa >>>>>>>>> >>>>>>>>> >>>>>>>>> On 2016/06/13 13:24, David Holmes wrote: >>>>>>>>> >>>>>>>>>> Hi Yasumasa, >>>>>>>>>> >>>>>>>>>> On 13/06/2016 1:45 PM, Yasumasa Suenaga wrote: >>>>>>>>>> >>>>>>>>>>> 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/ >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> It's okay to wait and hear what opinions others may have before >>>>>>>>>> changing things based on my comments. :) The fixed buffer may >>>>>>>>>> be okay - as I said I don't know what the potential options >>>>>>>>>> are, so don't know if it is okay or not. >>>>>>>>>> >>>>>>>>>> Using dynamic allocation avoids that but raises other concerns >>>>>>>>>> - like calling vm_exit_on_out_of_memory on failure; or whether >>>>>>>>>> to use malloc or resource area? >>>>>>>>>> >>>>>>>>>> Lets wait for other feedback before going further. >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> David >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> 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 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>> >>>>> >>>