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
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>>>>
>>>>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160504/7f32e568/attachment-0001.html>
More information about the serviceability-dev
mailing list