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

Yasumasa Suenaga yasuenag at gmail.com
Wed May 4 13:43:48 UTC 2016


Thanks, Marcus!

> into something like
>
> if (!success || values > SIZE_MAX) {

> Also, I think you probably need a static_cast here:
>
> 201 _rotate_size = value;

I applied them to new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8153073/webrev.07/


Thanks,

Yasumasa


On 2016/05/04 21:38, Marcus Larsson wrote:
> 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