PING: RFR: JDK-8153073: UL: Set filesize option with k/m/g

Marcus Larsson marcus.larsson at oracle.com
Wed May 4 12:38:44 UTC 2016


Hi,


On 05/04/2016 02:29 PM, Yasumasa Suenaga wrote:
> Hi David, Marcus,
>
> Thank you for your comment.
> I uploaded new webrev. Could you review it again?
>
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.06/

Can you join these two cases:

193 if (!success) {
194 break;
195 } else if (value > SIZE_MAX) {


into something like

if (!success || values > SIZE_MAX) {

so that we properly fail to configure the output and print the error 
message when atojulong fails as well?


Also, I think you probably need a static_cast here:

201 _rotate_size = value;

to avoid compiler warnings on 32-bit platforms.

Thanks,
Marcus

>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/05/04 15:49, Marcus Larsson wrote:
>> Hi,
>>
>> On 05/04/2016 03:45 AM, David Holmes wrote:
>>> On 4/05/2016 10:19 AM, Yasumasa Suenaga wrote:
>>>> Hi David, Marcus,
>>>>
>>>>> I would not have done that but instead used a temporary julong for 
>>>>> the
>>>>> parse result, then range check, then assign to the actual 
>>>>> _rotate_size
>>>>> etc with a cast.
>>>>
>>>> Thanks.
>>>> However, I guess we will encounter error when we access to > 2GB 
>>>> size file.
>>>> (Sorry, 4GB is incorrect.)
>>>
>>> That would seem to be a different issue.
>>>
>>>> I do not investigate for it yet. So I'm not sure whether it is 
>>>> correct.
>>>> In terms of this issue, should I keep range check and use julong 
>>>> temporary
>>>> value for _rotate_size?
>>>
>>> I would go this route yes. Use the Arguments code to do the parsing; 
>>> apply the same range check as exists today (which may or may not be 
>>> sufficient, but that is a different issue), then assign.
>>
>> I think that would be better too.
>>
>> Thanks,
>> Marcus
>>
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/05/04 5:40, David Holmes wrote:
>>>>> On 3/05/2016 11:49 PM, Yasumasa Suenaga wrote:
>>>>>>> This seems broken to me, we're passing &_rotate_size (a size_t*) to
>>>>>>> atojulong() which takes a julong*. If sizeof(julong) > 
>>>>>>> sizeof(size_t)
>>>>>>> then that's a problem, no?
>>>>>>
>>>>>> Thanks, Marcus!
>>>>>
>>>>> Yes good catch. I would have hoped the compiler would have complained
>>>>> about that on 32-bit.
>>>>>
>>>>>> I changed type of _rotate_size and _current_size to julong in new
>>>>>> webrev:
>>>>>
>>>>> I would not have done that but instead used a temporary julong for 
>>>>> the
>>>>> parse result, then range check, then assign to the actual 
>>>>> _rotate_size
>>>>> etc with a cast. That is less disruptive than changing the existing
>>>>> types IMHO.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.05/
>>>>>>
>>>>>>>> the question for the UL owners is whether the change in 
>>>>>>>> semantics is
>>>>>>>> appropriate: previously the filesize was interpreted as a value in
>>>>>>>> KB, whereas now, without a suffix, it is just bytes.
>>>>>>
>>>>>> Is it no problem?
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/05/03 21:41, Marcus Larsson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>
>>>>>>> On 05/03/2016 02:25 PM, David Holmes wrote:
>>>>>>>> Adding in the runtime team as they now own UL.
>>>>>>>>
>>>>>>>> I've reviewed the change from a coding perspective - the 
>>>>>>>> question for
>>>>>>>> the UL owners is whether the change in semantics is appropriate:
>>>>>>>> previously the filesize was interpreted as a value in KB, whereas
>>>>>>>> now, without a suffix, it is just bytes.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 3/05/2016 9:44 PM, Yasumasa Suenaga wrote:
>>>>>>>>> PING: Could you review and sponsor it?
>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.04/
>>>>>>>
>>>>>>> This seems broken to me, we're passing &_rotate_size (a size_t*) to
>>>>>>> atojulong() which takes a julong*. If sizeof(julong) > 
>>>>>>> sizeof(size_t)
>>>>>>> then that's a problem, no?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Marcus
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2016/04/21 18:37, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi David,
>>>>>>>>>>
>>>>>>>>>>> So it just registered with me that currently filesize is
>>>>>>>>>>> interpreted
>>>>>>>>>>> as a value in KB. With this change it will be in bytes - 
>>>>>>>>>>> that means
>>>>>>>>>>> tests will need fixing eg:
>>>>>>>>>>>
>>>>>>>>>>> hotspot/test/serviceability/logging/TestLogRotation.java
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> I've fixed it in new webrev:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.04/
>>>>>>>>>>
>>>>>>>>>> Following jtreg tests are passed:
>>>>>>>>>>
>>>>>>>>>>  - hotspot/test/gc/logging
>>>>>>>>>>  - hotspot/test/runtime/logging
>>>>>>>>>>  - hotspot/test/serviceability/logging
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2016/04/21 14:43, David Holmes wrote:
>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>
>>>>>>>>>>> On 20/04/2016 7:15 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>
>>>>>>>>>>>>> ... on 32-bit size_t and julong are not the same size so 
>>>>>>>>>>>>> we would
>>>>>>>>>>>>> still need to ensure we don't specify a filesize that is 
>>>>>>>>>>>>> greater
>>>>>>>>>>>>> than
>>>>>>>>>>>>> SIZE_MAX on 32-bit.
>>>>>>>>>>>>
>>>>>>>>>>>> Oh... I understood.
>>>>>>>>>>>> I've fixed and uploaded new webrev. Could you review again?
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.03/
>>>>>>>>>>>
>>>>>>>>>>> So it just registered with me that currently filesize is
>>>>>>>>>>> interpreted
>>>>>>>>>>> as a value in KB. With this change it will be in bytes - 
>>>>>>>>>>> that means
>>>>>>>>>>> tests will need fixing eg:
>>>>>>>>>>>
>>>>>>>>>>> hotspot/test/serviceability/logging/TestLogRotation.java
>>>>>>>>>>>
>>>>>>>>>>> That change in semantics may not be desirable, but I'll leave
>>>>>>>>>>> that to
>>>>>>>>>>> the owners of this code to decide (and I hope they jump in 
>>>>>>>>>>> soon!)
>>>>>>>>>>>
>>>>>>>>>>> I note that in the existing range check:
>>>>>>>>>>>
>>>>>>>>>>>   if (value == SIZE_MAX || value > SIZE_MAX / K) {
>>>>>>>>>>>
>>>>>>>>>>> the first clause is redundant. So your change seems okay.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>> -----
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 2016/04/20 15:04, David Holmes wrote:
>>>>>>>>>>>>> On 20/04/2016 3:25 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> Hi David,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>  > You still removed the size bounds checks:
>>>>>>>>>>>>>>  >
>>>>>>>>>>>>>>  >  190       size_t value = parse_value(value_str);
>>>>>>>>>>>>>>  >  191       if (value == SIZE_MAX || value > SIZE_MAX / 
>>>>>>>>>>>>>> K) {
>>>>>>>>>>>>>>  >  192 errstream->print_cr("Invalid option: %s must
>>>>>>>>>>>>>> be in
>>>>>>>>>>>>>> range
>>>>>>>>>>>>>> [0, "
>>>>>>>>>>>>>>  >  193 SIZE_FORMAT "]",
>>>>>>>>>>>>>> FileSizeOptionKey,
>>>>>>>>>>>>>> SIZE_MAX / K);
>>>>>>>>>>>>>>  >  194         success = false;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> SIZE_MAX is defined as ULONG_MAX in stdint.h [1].
>>>>>>>>>>>>>
>>>>>>>>>>>>> Ah I hadn't realized this was an external value, I thought it
>>>>>>>>>>>>> was some
>>>>>>>>>>>>> internally enforced maximum file size limit. So this is 
>>>>>>>>>>>>> just an
>>>>>>>>>>>>> overflow check really, and ...
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Arguments::atojulong(atomull) checks value range [2].
>>>>>>>>>>>>>
>>>>>>>>>>>>> ... we already do an overflow check in here, but ...
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thus I do not think we do not need to check value range in
>>>>>>>>>>>>>> LogFileOutput::parse_options().
>>>>>>>>>>>>>
>>>>>>>>>>>>> ... on 32-bit size_t and julong are not the same size so 
>>>>>>>>>>>>> we would
>>>>>>>>>>>>> still need to ensure we don't specify a filesize that is 
>>>>>>>>>>>>> greater
>>>>>>>>>>>>> than
>>>>>>>>>>>>> SIZE_MAX on 32-bit.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>  > Thanks, I had missed that example usage buried in there,
>>>>>>>>>>>>>> but am
>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>  > surprised none of these "options" for the handling the
>>>>>>>>>>>>>> file are
>>>>>>>>>>>>>>  > explicitly documented.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I do not know how we can documented about it.
>>>>>>>>>>>>>> (Is it internal process in Oracle?)
>>>>>>>>>>>>>
>>>>>>>>>>>>> No I just meant that amongst all that help text you already
>>>>>>>>>>>>> modified,
>>>>>>>>>>>>> there is nothing, that I could see, that actually 
>>>>>>>>>>>>> describes the
>>>>>>>>>>>>> possible options for filesize.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> David
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I can help for it if I can
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>> https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/generic/stdint.h;h=442762728b899aa8ec219299692fce5953d796b0;hb=HEAD#l259 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [2]
>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/8005261869c9/src/share/vm/runtime/arguments.cpp#l804 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2016/04/20 11:24 "David Holmes" <david.holmes at oracle.com
>>>>>>>>>>>>>> <mailto:david.holmes at oracle.com>>:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     Hi Yasumasa,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     On 19/04/2016 11:50 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      > Hi David,
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > Thank you for your comment.
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > I uploaded new webrev. Could you review again?
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.02/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     You still removed the size bounds checks:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>       190       size_t value = parse_value(value_str);
>>>>>>>>>>>>>>       191       if (value == SIZE_MAX || value > SIZE_MAX 
>>>>>>>>>>>>>> / K) {
>>>>>>>>>>>>>>       192 errstream->print_cr("Invalid option: %s must
>>>>>>>>>>>>>> be in
>>>>>>>>>>>>>>     range [0, "
>>>>>>>>>>>>>>       193 SIZE_FORMAT "]",
>>>>>>>>>>>>>> FileSizeOptionKey,
>>>>>>>>>>>>>>     SIZE_MAX / K);
>>>>>>>>>>>>>>       194         success = false;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>      >> Which makes me wonder if atomull needs renaming -
>>>>>>>>>>>>>> does the
>>>>>>>>>>>>>> "m" mean
>>>>>>>>>>>>>>      >> memory? atojulong would seem more appropriate
>>>>>>>>>>>>>> regardless.
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > I renamed to atojulong() in new webrev.
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      >> Not directly related to your change but I was 
>>>>>>>>>>>>>> surprised
>>>>>>>>>>>>>> that the
>>>>>>>>>>>>>>      >> various log file options don't seem to be documented
>>>>>>>>>>>>>> anywhere
>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>      >> -Xlog:help output.
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > I updated help message in new webrev.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     Thanks, I had missed that example usage buried in there,
>>>>>>>>>>>>>> but am
>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>     surprised none of these "options" for the handling 
>>>>>>>>>>>>>> the file
>>>>>>>>>>>>>> are
>>>>>>>>>>>>>>     explicitly documented.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     Thanks,
>>>>>>>>>>>>>>     David
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > Thanks,
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > Yasumasa
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      >
>>>>>>>>>>>>>>      > On 2016/04/19 10:14, David Holmes wrote:
>>>>>>>>>>>>>>      >> Hi Yasumasa,
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> On 19/04/2016 12:06 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      >>> PING:
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> I've sent review request for JDK-8153073.
>>>>>>>>>>>>>>      >>> Could you review it?
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> If this patch is merged, user can set logfile 
>>>>>>>>>>>>>> size with
>>>>>>>>>>>>>> k/m/g.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> Your webrev seems out of date with respect to the
>>>>>>>>>>>>>> current
>>>>>>>>>>>>>> code - the
>>>>>>>>>>>>>>      >> logfile size processing is done in
>>>>>>>>>>>>>> LogFileOutput::parse_options not
>>>>>>>>>>>>>>      >> configure_rotation. And of course you now need to 
>>>>>>>>>>>>>> work
>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>     jdk9/hs not
>>>>>>>>>>>>>>      >> hs-rt.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> That aside:
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> src/share/vm/runtime/arguments.cpp
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> I don't think you need to add the Arguments:: to the
>>>>>>>>>>>>>> atomull
>>>>>>>>>>>>>>     calls when
>>>>>>>>>>>>>>      >> you are executing in Arguments code - lines 2643, 
>>>>>>>>>>>>>> 2660
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> This comment could be updated to delete "memory"
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >>    788 // Parses a memory size specification string.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> Which makes me wonder if atomull needs renaming -
>>>>>>>>>>>>>> does the
>>>>>>>>>>>>>> "m" mean
>>>>>>>>>>>>>>      >> memory? atojulong would seem more appropriate
>>>>>>>>>>>>>> regardless.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> ---
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> src/share/vm/logging/logFileOutput.cpp
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> You removed the size checking logic.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> Not directly related to your change but I was 
>>>>>>>>>>>>>> surprised
>>>>>>>>>>>>>> that the
>>>>>>>>>>>>>>      >> various log file options don't seem to be documented
>>>>>>>>>>>>>> anywhere
>>>>>>>>>>>>>> in the
>>>>>>>>>>>>>>      >> -Xlog:help output.
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >> Thanks,
>>>>>>>>>>>>>>      >> David
>>>>>>>>>>>>>>      >> -----
>>>>>>>>>>>>>>      >>
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> Please review it.
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> Thanks,
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> Yasumasa
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>>
>>>>>>>>>>>>>>      >>> On 2016/04/11 18:28, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      >>>> PING: Could you review it?
>>>>>>>>>>>>>>      >>>> We need more reviewer.
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>> Thanks,
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>> Yasumasa
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>>
>>>>>>>>>>>>>>      >>>> On 2016/03/31 22:33, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      >>>>> CC'ed to serviceability-dev.
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>> Could you review it?
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>> Thanks,
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>> Yasumasa
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>>
>>>>>>>>>>>>>>      >>>>> On 2016/03/31 18:24, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      >>>>>> Hi Marcus,
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>>> You're missing an include of arguments.hpp in
>>>>>>>>>>>>>>     logFileOutput.cpp.
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>> arguments.hpp is included in precompiled.hpp 
>>>>>>>>>>>>>> . So
>>>>>>>>>>>>>> build was
>>>>>>>>>>>>>>     succeeded.
>>>>>>>>>>>>>>      >>>>>> However, it should be included in
>>>>>>>>>>>>>> logFileOutput.cpp .
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>> I uploaded a new webrev. Could you review again?
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.01/
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>> Thanks,
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>> Yasumasa
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>>
>>>>>>>>>>>>>>      >>>>>> On 2016/03/31 16:48, Marcus Larsson wrote:
>>>>>>>>>>>>>>      >>>>>>> Hi,
>>>>>>>>>>>>>>      >>>>>>>
>>>>>>>>>>>>>>      >>>>>>> On 03/30/2016 04:09 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>      >>>>>>>> Hi all,
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>> This request review is related to [1].
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>> I want to set filesize option with k/m/g as 
>>>>>>>>>>>>>> below:
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>> -Xlog:gc=trace:file=gc.log:time:filecount=5,filesize=10m
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>> Memory size option (e.g. -Xmx) can be set with
>>>>>>>>>>>>>> k/m/g .
>>>>>>>>>>>>>>      >>>>>>>> I think we can use option parser in
>>>>>>>>>>>>>> arguments.cpp .
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>> I uploaded webrev. Could you review it?
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.00/
>>>>>>>>>>>>>>      >>>>>>>
>>>>>>>>>>>>>>      >>>>>>> You're missing an include of arguments.hpp in
>>>>>>>>>>>>>>     logFileOutput.cpp.
>>>>>>>>>>>>>>      >>>>>>>
>>>>>>>>>>>>>>      >>>>>>> Apart from that, this looks good to me.
>>>>>>>>>>>>>>      >>>>>>>
>>>>>>>>>>>>>>      >>>>>>> Thanks,
>>>>>>>>>>>>>>      >>>>>>> Marcus
>>>>>>>>>>>>>>      >>>>>>>
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>>
>>>>>>>>>>>>>>      >>>>>>>> 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