PING: RFR: 8217432: MetaspaceGC::_capacity_until_GC exceeds MaxMetaspaceSize
Yasumasa Suenaga
yasuenag at gmail.com
Wed Jan 30 13:16:36 UTC 2019
Thanks Thomas!
I uploaded new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8217432/webrev.02/
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?
Thanks,
Yasumasa
On 2019/01/30 21:55, Thomas Schatzl wrote:
> On Wed, 2019-01-30 at 21:49 +0900, Yasumasa Suenaga wrote:
>> Hi Thomas,
>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8217432/webrev.01/
>>>>
>>>> I write reply based on it as below.
>>
>>>> I added the code to check total amount of _capacity_until_GC
>>>> before inc_capacity_until_GC() call.
>>>> But I concern the check and increment are not atomically. Is it
>>>> okay?
>>>
>>> If the check were located after the Atomic::cmpxchg() and after the
>>> check whether it had been this thread to change the value it would
>>> be
>>> fine :)
>>
>> Do you suggest as following code?
>
> No, in MetaspaceGC::inc_capacity_until_GC() after the cmpxchg and the
> check whether this thread has been the one incrementing the watermark.
>
> I.e. "
>
> size_t prev_value = Atomic::cmpxchg(new_value, ...);
>
> if (old_capacity_until_gc != prev_value) {
> return false; // Somebody else incremented the counter
> }
>
> assert(new_value <= MaxMetaspaceSize, "...
>
> "
>
> At that point in execution we are sure this thread set "new_value" to a
> new value, that should obviously not exceed MaxMetaspaceSize.
>
> Please use a meaningful assert text containing the increment, new value
> and threshold.
>
> Thanks,
> Thomas
>
>
More information about the hotspot-gc-dev
mailing list