PING: RFR: 8217432: MetaspaceGC::_capacity_until_GC exceeds MaxMetaspaceSize
thomas.stuefe at gmail.com
Fri Feb 1 09:25:59 UTC 2019
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
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
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.
> 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...
More information about the hotspot-gc-dev