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

[Erik Österlund]

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. 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. 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. 
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. It cannot
flicker any more at this point, which makes the first part of the 
condition checking that the _owner is deflating redundant,
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.

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?

/Erik

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