[master] RFR: Simplify stable_mark() routine [v2]

[Roman Kennke]

On Wed, 13 Apr 2022 09:28:04 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Hi,
> 
> looked through this and also the old version (I had missed that) and it looks fine. Interesting trick of temporary set the mark to "installing". I guess there is no code being surprised by the new state change (installing->locked)? Or could that happen before too?

I haven't found any other code that would be surprised by this. That's probably because any such code would have to deal with it somehow already, when doing real monitor inflation. The trick has been introduced much earlier though, in #12 and later improved in #25, this PR only shuffles code around to be more compact and more efficient.

> So, the old version returned true for "monitor" (10) and for "marked" (11) both? Seems like a bug. Would it be worth fixing it independently, maybe upstream?

Yes. Maybe. All existing code seems to dodge this problem by either testing in a specific order, or not being affected by the marked state at all. It seems brittle and pointless to not do it correctly, though, so yeah, I'll propose it upstream.

> src/hotspot/share/runtime/synchronizer.cpp line 852:
> 
>> 850:       object->release_set_mark(mark);
>> 851: 
>> 852:       return dmw;
> 
> Question, why is above guarantee a guarantee, not an assert? You want to be super sure here also in release builds? Do you plan to drop down to assert() eventually?

TBH, dunno really. This code originates from ObjectSynchronizer::inflate() and we can probably turn it into an assert.

-------------

PR: https://git.openjdk.java.net/lilliput/pull/46