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

[Erik Österlund]

Hi Roman,

On 2018-07-26 16:02, Roman Kennke wrote:
> Am 26.07.2018 um 15:13 schrieb Erik Österlund:
>> Hi Roman,
>>
>> On 2018-07-26 13:09, Roman Kennke wrote:
>>> 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.
>> The reason is that wherever ON_UNKNOWN_OOP_REF is used, the backend
>> needs to be able to determine the exact strength. And to do that, the
>> backend needs to be able to determine of this is a referent field. And
>> to do that, it needs a base pointer.
>>
>> I'm not 100% sure what I think is a good solution to this. I wonder if
>> along the lines of introducing these resolve for read/write decorators
>> (which it looks like we will be needing anyway), there could be a do not
>> resolve decorator that could be passed in to determining how to resolve
>> the access. Default for stores could be ACCESS_WRITE, for loads
>> ACCESS_READ, for atomics ACCESS_READ | ACCESS_WRITE, and explicitly
>> setting ACCESS_NONE meaning don't resolve this one. Maybe the prefix
>> ought to be RESOLVE_READ / RESOLVE_WRITE / RESOLVE_NONE instead though
>> to be more specific.
> We are in instanceRefKlass, and we should be able to determine the
> reference strength statically, and pass in the correct ON_XXX_OOP_REF
> decorator, right? E.g. via InstanceKlass::reference_type() ? Or would
> that not work?

That should probably do the trick, yes.

/Erik