Why is mark read as MO_RELAXED in read_stable_mark()?

[Roman Kennke]

>> Hi Hotspot runtime devs,
>>
>> I am messing a little bit with read_stable_mark() in synchronizer.cpp 
>> in Lilliput project (because I want to load the Klass* from the header).
>>
>> I notice that, in read_stable_mark(), we are loading the header with 
>> MO_RELAXED memory ordering, even though the INFLATING header is stored 
>> either via CAS or (re-)stored with MO_RELEASE. Wouldn't it be more 
>> consequential to load the header with MO_ACQUIRE instead?
> For the monitor case I think we already have an address dependency in 
> the reader so there is no need for the acquire. So we first read the 
> monitor address and then use that address to access the _header 
> field(FastHashCode() case) or _owner field(current_thread_holds_lock() 
> and get_lock_owner() cases). The writer in turn orders the stores to 
> _owner/_header and the publishing of the monitor address by using a 
> release store in inflate() (I think this is the one you are referring to).
> 

Thanks Patricio,

I am not quite sure that I follow you.

On the reader side, given an object, when a thread calls into 
FastHashCode() or any locking that causes inflation, it needs to read 
the object header, and that is always done with a plain read. In 
particular, read_stable_mark() does that, while waiting for the header 
to become != INFLATING. On the writer side, we use CAS (using 
MO_SEQ_CST) to temporarily turn on INFLATING(with release_store() to 
write back the original header, and store with MO_RELEASE to write back 
the original header.

It probably doesn't matter all that much.

I am wondering about this because I am currently moving the Klass* into 
the object header, which means that for klass() implementation, we need 
to load the header similar to how we do it for hashcode(). In particular 
it involves read_stable_mark(). Now, there is a variant of 
klass-accessor that implies acquire-semantics: 
oopDesc::klass_or_null_acquire(). I am not quite sure why exactly we 
need acquire semantics there, but in order to implement it, I believe we 
would have to make a variant of read_stable_mark() that uses acquire 
semantics to load the header, instead of plain reads.

But I'm thinking it may be cleaner overall to always use acquire 
semantics in read_stable_mark() (mirroring the release-store and 
seq-cst-CAS), which would make the acquire-accessors for klass() 
superfluous.

What do you think?