UL: Two enhancement proposal for UL

[Marcus Larsson]

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
>
>