RFR(L) 8153224 Monitor deflation prolong safepoints (CR11/v2.11/14-for-jdk15)

[Daniel D. Daugherty]

On 5/15/20 11:23 AM, Erik Österlund wrote:
>
>
> On 2020-05-15 17:01, Daniel D. Daugherty wrote:
>> On 5/15/20 9:17 AM, Erik Österlund wrote:
>>> Replying to myself here.
>>>
>>> On 2020-05-15 13:07, Erik Österlund wrote:
>>>> Oh and we probably need a normal loadload() between reading _owner 
>>>> and _contentions in the is_async_deflating function. The
>>>> loads re-ordering implies we could spuriously read an old value for 
>>>> _owner (not deflating) and a new value for _contentions, leading it
>>>> to believe it isn't deflating even though it is. But that is easier 
>>>> to reason about because the loads are reading values written by the 
>>>> same thread.
>>>>
>>>
>>> On that note, why does is_async_deflating() check both the _owner 
>>> and _contentions? It requires both
>>> to be updated to indicate deflation. But it seems to me that 
>>> checking only _contentions is enough.
>>>
>>> My understanding of the algorithm that making _contentions negative 
>>> is the linearization point at which
>>> we have decided that the monitor is async deflating and should no 
>>> longer be used. The deflation marker
>>> in the owner field is just the thing we poke in there before 
>>> reaching the linearization point to force fast paths
>>> that normally don't check _contentions to take a slow path instead, 
>>> that checks it.
>>>
>>> If _contentions < 0, then it is implied that _owner == deflating 
>>> (and we assert that in some places).
>>> It seems to me that the only thing checking the owner does here is 
>>> to cause headache and spurious false
>>> negatives due to requiring the careful fencing I described in order 
>>> to return precisely the same result we
>>> would get by looking only if the _contentions counter is negative. 
>>> Or am I missing something? Is there a
>>> valid case where the _contentions is negative, yet the _owner is not 
>>> deflating, that we need to catch
>>> and distinguish? I think not, and I worry about some other asserts 
>>> if there truly is.
>>
>> You have forgotten about the A-B-A race where a racing thread can enter
>> an ObjectMonitor after the deflater thread sets the owner field and 
>> before
>> the deflater thread can change contentions.
>>
>> There's a whole subsection on it in the wiki:
>>
>> https://wiki.openjdk.java.net/display/HotSpot/Async+Monitor+Deflation#AsyncMonitorDeflation-T-enterWinsByA-B-A 
>>
>
> No I did not forget about that.

Then please read it again.


> My point is still the same: the owner field can be a bit indecisive 
> about whether
> deflation might happen soon or not. But the linearization point where 
> we say this monitor is definitely toast now,
> and there is no going back, is when _contentions is negative.

No that is not the linearization point. The linearization point is when
the owner field == DEFLATER_MARKER and the contentions field is negative.
Both of those must be true in order for async deflation to proceed. This
is the KEY check in the async deflation algorithm.

Due to the A-B-A race described above, it is possible for the contentions
field to be negative and the owner field to be != DEFLATER_MARKER. If you
relied only on the contentions field being negative, then the A-B-A race
described above would mean that you have one thread that thinks it has
successfully entered the monitor and the deflater thread thinking that it
has successfully async deflated the monitor. Chaos would ensure.


> That's why I am saying that it makes sense for
> is_async_deflating() to look exclusively at whether _contentions is 
> negative or not, and not what the owner field says.
> If _contentions is negative, then _owner *must* be the deflation marker.

Not true. That last sentence ignores the A-B-A race where contentions is
negative AND the owner field has changed from DEFLATER_MARKER to the new
owner of the monitor.

The caller of is_async_deflating() can be an observer of the race between
the deflating thread and the entering thread. The only way that the
observer can be sure that the ObjectMonitor is async deflating is:

     (owner_is_DEFLATER_MARKER() && contentions() < 0)


> And if it isn't negative, then we must return
> false, because it is not yet definitive that this monitor will 
> deflate, due to the exact race you pointed out.

Observation of a negative value in contentions is not definitive because
the owner field could have been changed from DEFLATER_MARKER to the new
owner at the same time that the deflater thread is setting the contentions
field to a negative value. That is the very nature of this A-B-A race.


> It cannot
> flicker any more at this point,

Not true.


> which makes the first part of the condition checking that the _owner 
> is deflating redundant,

Also not true.


> yet a source of data races due to missing fencing requirements that 
> obscures the result to sometimes be wrong, as
> there is no interleaving loadload() that ensures that _contentions < 0 
> really must imply that _owners == deflating is true,
> but only due to the lack of fencing.

Sigh... contentions < 0 does not imply owner == DEFLATER_MARKER. It might
be true if we don't have a racing enter, but it might not. That's the whole
point of the A-B-A race.


> Due to the mental overhead of thinking about what fencing is right 
> here, it occured to me that just removing the
> _owner check will exactly reflect the linearization point when the 
> monitor becomes is_async_deflating(), without having
> to reason about the fencing w.r.t. the _owner field that we don't 
> really care about what it is.
>
> Or did I miss something?

Yes. You've somehow missed the whole point of the A-B-A race.

Dan

>
> /Erik
>
>> Dan
>>
>>
>>>
>>> Thanks,
>>> /Erik
>>
>