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

Thomas Schatzl thomas.schatzl at
Wed Jan 30 14:55:53 UTC 2019


On Wed, 2019-01-30 at 22:16 +0900, Yasumasa Suenaga wrote:
> Thanks Thomas!
> I uploaded new webrev:
> But I guess we can remove the following change:
> --------------
> @@ -1477,6 +1483,9 @@
>     // the HWM, an allocation is still attempted. This is because
> another thread must then
>     // have incremented the HWM and therefore the allocation might
> still succeed.
>     do {
> +    if (MetaspaceGC::capacity_until_GC() + delta_bytes >
> MaxMetaspaceSize) {
> +      return NULL;
> +    }
>       incremented = MetaspaceGC::inc_capacity_until_GC(delta_bytes,
> &after, &before);
>       res = allocate(word_size, mdtype);
>     } while (!incremented && res == NULL);
> --------------
> OTOH inc_capacity_until_GC() shouldn't be passed the value which
> exceeds MaxMetaspaceSize.
> Should we keep this change?

Actually this code (either my suggestion or yours) does not work either
- if we read capacity_until_GC() here and do the check, in the re-read
in inc_capacity_until_gc() to calculate the new value may be different
(may have been updated in the meantime).

I was not looking at the loop in detail which is broken as well,
assuming like other code we just fixed that allocation is always

One option would be to have inc_capacity_until_gc() return an
additional bool value whether MaxMetaspaceSize has already been
exceeded (i.e. there is a chance that any further attempt is
successful), and exit this this loop if not so.

This is different to the initial approach where the method returned
false as result because the code can fail to set a new value due to
races anyway, but still be able to retry.

Another option would be to have inc_capacity_until_gc() return true to
fake a successful increment if MaxMetaSpaceSize had already been
reached. This is a bit sneaky though so I would prefer the former
option. What do you think?


More information about the hotspot-gc-dev mailing list