RFR: JDK-8206457: Code paths from oop_iterate() must use barrier-free access

[Roman Kennke]

Hi Erik,
thanks for reviewing!

> In instanceRefKlass.inline.hpp:
> 
> Are these changes to logging required for Shenandoah not to crash? It
> appears to me that for ZGC, it would print the wrong addresses if
> barriers were not used. That's why I wonder if this is a strict
> requirement or not for Shenandoah to work.

We do need to avoid read-barriers on those paths. The problem is that
during full-GC we temporarily don't have the fwd-ptr available (it's a
sliding mark-compact algorithm). However, we can work around this by not
using the base+offset variants like in the patch. However, this seems to
make the Access API unhappy at compile-time when using
ON_UNKNOWN_OOP_REF. Can you check this? I've no clue where to look.

> In oops/oop.cpp:
> Rather than changing the metadata accessors to always assume raw
> accesses are enough for all metadata accesses, I would prefer to have
> explicit metadata_field_raw accessors instead where we expect this (even
> if that turns out to be always).

Alright, I unwinded it and came up with a minimal use of
metadata_field_raw(). It requires splicing Class:as_Klass() into a
_raw() variant to be used from oop iterator paths. But it's not as bad
as I thought.

Updated patch:
http://cr.openjdk.java.net/~rkennke/JDK-8206457/webrev.01/

Do you like it better? Can you check the compile-error that I mentioned
above?

Thanks, Roman


> 
> Thanks,
> /Erik
> 
> On 2018-07-06 16:46, Roman Kennke wrote:
>>   We have several code paths going out from oop_iterate() methods that
>> lead to GC barriers. This is not only inefficient but outright wrong.
>> oop_iterate() is normally used by GC and GC need to see the raw stuff,
>> not some resolved objects. In Shenandoah's full-GC it's fatal to attempt
>> to read objects's forwarding pointers, because it's temporarily pointing
>> to nowhere land.
>>
>> I propose to selectively use _raw() variants of the various accessors
>> that are used on oop_iterate() paths. This means to introduce an
>> oopDesc::int_field_raw(). I also propose to change metadata_field()
>> accessors to always use raw access wholesale. This is only used to load
>> the Klass* field, which is immutable and thus doesn't require barriers.
>>
>> The log_* statements in instanceRefKlass.inline.hpp surely don't need
>> barriers. I turned them into raw accessors as well.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8206457?filter=-1
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8206457/webrev.00/
>>
>> Test: passes hotspot-tier1 here.
>>
>> Can I please get review?
>>
>> Roman
>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180726/2368c6df/signature.asc>