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

Roman Kennke rkennke at redhat.com
Thu Aug 9 08:10:40 UTC 2018



>>>>>> 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



More information about the hotspot-gc-dev mailing list