PING: RFR: 8217432: MetaspaceGC::_capacity_until_GC exceeds MaxMetaspaceSize

[Yasumasa Suenaga]

Thanks Thomas!

I will change the comment.


Yasumasa



2019年2月1日(金) 18:26、Thomas Stüfe さん（thomas.stuefe at gmail.com）のメッセージ:

> Hi Yasumasa,
>
> looks good to me as well.
>
> I like the added comment but would suggest making it more precise: "Try to
> increase metaspace size by v bytes" reads like it does anything of
> substance like allocating memory etc, but it does nothing but increase the
> limit variable. How about this:
>
> "Try to increase the _capacity_until_GC limit counter by v bytes.
>  Returns true if it succeeded. It may fail if either another thread
> concurrently increased the limit or the new limit would be larger than
> MaxMetaspaceSize."
>
> Maybe also indicate that can_retry is only defined if function returns
> false, new_cap_until_GC and old_cap_until_GC only if return value is true:
>
> - // Optionally returns new and old metaspace capacity in
> + // On success, optionally returns new and old metaspace capacity in
>
> - // Optionally sets can_retry to indicate whether if there is actually
> + // On error, optionally sets can_retry to indicate whether if there is
> actually
>
> If you change the comment, I am fine with the patch as it is and do not
> need another webrev.
>
> Thank you, Thomas
>
>
>
>
>
>
> On Thu, Jan 31, 2019 at 3:00 PM Yasumasa Suenaga <yasuenag at gmail.com>
> wrote:
>
>> Thanks Thomas!
>>
>> I will add the comment to inc_capacity_until_GC() declaration.
>>
>> I'm waiting for second reviewer.
>>
>>
>> Yasumasa
>>
>>
>> On 2019/01/31 18:16, Thomas Schatzl wrote:
>> > Hi Yasumasa,
>> >
>> > On Thu, 2019-01-31 at 14:09 +0900, Yasumasa Suenaga wrote:
>> >> Hi Thomas,
>> >>
>> >> I uploaded new webrev:
>> >>    http://cr.openjdk.java.net/~ysuenaga/JDK-8217432/webrev.03/
>> >>
>> >> I agree with you that inc_capacity_until_GC() returns additional
>> >> bool value whether new HWM is exceeded MaxMetaspaceSize.
>> >>
>> >> I added `can_retry` to argument of inc_capacity_until_GC().
>> >> It will be set to false if `new_value` exceeds MaxMetaspaceSize.
>> >> Then inc_capacity_until_GC() returns false, but it is not break
>> >> assert() in compute_new_size() because inc_capacity_until_GC()
>> >> is called with `expand_bytes` which is limited by MaxMetaspaceSize.
>> >> (It is ensured by this change)
>> >>
>> >> This change has passed vmTestbase/metaspace, gc/metaspace, and
>> >> submit repo tests.
>> >
>> >    looks good to me. Can you add some documentation to the declaration
>> > of inc_capacity_until_GC() like:
>> >
>> > // Try to increase metaspace size by v bytes. Returns true if
>> > // succeeded, false if not due to competing threads trying.
>> > // Optionally returns new and old metaspace capacity in
>> > // new_cap_until_GC and old_cap_until_GC respectively.
>> > // Optionally sets can_retry to indicate whether if there is actually
>> > // enough space remaining to satisfy the request.
>> >
>> > No need for a re-review for that (or something similar potentially
>> > better worded description.
>> >
>> > Thanks,
>> >    Thomas
>> >
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190201/3cf85065/attachment.htm>