[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: UL: Two enhancement proposal for UL
From:       Yasumasa Suenaga <yasuenag () gmail ! com>
Date:       2016-03-30 14:11:30
Message-ID: 56FBDE92.4050105 () gmail ! com
[Download RAW message or body]

Hi Marcus,

Thank you for replying.

I filed them to JBS and sent review request:

  http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-March/018710.html
  http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-March/018711.html


Thanks,

Yasumasa


On 2016/03/30 18:34, Marcus Larsson wrote:
> Hi Yasumasa,
> 
> On 03/29/2016 04:43 PM, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> I want to propose two enhancement for UL.
>>
>>
>>    1. Set filesize option with k/m/g
>>         I want to set filesize option with k/m/g:
>>            -Xlog:gc=trace:file=gc.log:time:filecount=5,filesize=10m
>>
>>         Implementation sample:
>>            http://cr.openjdk.java.net/~ysuenaga/ul/set-filesize-option/
> 
> In logFileOutput.cpp:
> 
> 119 size_t value;
> 120 if (Arguments::atomull(value_str, &value)) {
> 121 if (value <= SIZE_MAX) {
> 122 _rotate_size = value;
> 123 success = true;
> 124 } else {
> 125 success = false;
> 126 }
> 127 break;
> 128 } else {
> 
> The if-statement on line 121 is redundant and should be removed. You
> could also let atomull write directly to the _rotate_size field and
> remove the local size_t value.
> 
> Other than that the patch looks good to me.
> 
>>
>>
>>    2. Show output option in VM.log jcmd
>>         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,
>>
>>         Implementation sample:
>>            http://cr.openjdk.java.net/~ysuenaga/ul/show-output-options/
> 
> I think this is a good idea, but the implementation could be simpler.
> Since we don't allow reconfiguration of output options there's no need
> to set the option string anywhere else but in the constructor. This
> removes the need for a set_option_string method as long as we keep the
> _option_string protected so that subclasses may set it in their
> constructor. Also, the destructor in LogOutput is missing
> os::free(_output_string).
> 
>>
>>
>> I want to file them to JBS and to upload webrev.
>> Are they accepted?
> 
> I think they're fine.
> 
> Thanks,
> Marcus
> 
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
> 
[prev in list] [next in list] [prev in thread] [next in thread]