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

[Erik Österlund]

Hi Roman,

Yes, looks good.

Thanks,
/Erik

On 2018-08-09 10:10, 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.
>>> not 100% sure this is the correct ReferenceType -> decorators mapping?
>>>
>>> Incremental:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8206457/webrev.02.diff/
>>> Full patch:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8206457/webrev.02/
>>>
>> Ping?
>
> Erik, is this good now?
>
> Thanks, Roman