[External] : Re: Semantics of CollectedHeap::requires_barriers()

[Ron Pressler]

> On 8 Feb 2021, at 12:23, Roman Kennke <rkennke at redhat.com> wrote:
> 
> Hi Ron,
> 
> Thanks for the explanation!
> 
> Does that mean that always returning true in requires_barriers() should be conservatively ok? If yes, why am I hitting this assert:
> 
> https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/e8aeb3d34d9dee2f1f5c1ddc71a41adc991b5808/src/hotspot/share/runtime/continuation.cpp*L2650__;Iw!!GqivPVa7Brio!JdCI5e_oUvM0vddq0EEq1183zGwMWEwdly7p6oiE96kvHzFrUSpysBL5F8JJu1Migg$ 
> ?

No, you must not always return true. An that’s just been allocated since the last safepoint must return false.
Otherwise, we’d never be able to freeze — we can only freeze frames into a chunk that doesn’t require barriers.

> 
> Besides this, I think I know what to do for requires_barriers() in Shenandoah. In-fact (if I understand it corrcetly) I don't think Shenandoah needs any barriers at all there: this is about copying stack-frames (which might contain oops) *into* the StackChunk. It should be ok for SATB because there are no previous oop locations in a new StackChunk, and it should be ok for LRB because LRBs are only relevant when *loading* from oop locations, but here we're storing to oop locations.

First, when storing frames, we already require that no barriers are needed. If they are we can’t freeze (i.e. we can never freeze into a chunk
whose requires_barriers returns true).

Now, we are reusing the same chunk for multiple freezes, as long as it doesn’t require barriers. That means we are overwriting oops,
which is not okay for SATB. Unless you perform the writes in memory that has been allocated after marking started. Such memory does not 
need to be marked through by SATB marking. 

We must not change the oop layout of something that is concurrently being traversed by the GC. Obviously if you allow to overwrite the chunk at 
any time, that implies it will get overwritten by a marking traversal operation that runs concurrently and inevitably crash. So it's not valid to mutate 
the layout or contents of a stack chunk that can be traversed concurrently by the GC, which again takes us back to allocating memory (allocated 
since the last marking phase started), which is not traversed by concurrent marking.

> 
> However, we might need barriers when copying frames out of the stack chunk (e.g. when thawing frames), because then we'd be loading from oop locations. Is this taken care of already somehow?

If you need barriers when thawing, then requires_barriers must return false, and then it’s taken care of.

> 
> Similar considerations should apply for ZGC. Or maybe I'm missing something?

ZGC doesn’t require barriers iff the object is in an allocating region. Something similar would need to be done for Shenandoah, I assume.
If Shenandoah *always* requires barriers when reading, then we might need to split requires_barriers into two different methods.

> 
> Do GCs need to treat StackChunk instances specially when marking or relocating?

Absolutely, but I think that’s done automatically, as InstanceStackChunkKlass overrides oop_oop_iterate etc.

> 
> I am a bit skeptical about ContMirror::allocate_stack_chunk() (in continuation.cpp around line 5965), it looks to me like it's trying to second-guess GC allocation mechanics by going directly to the TLAB. I'm not sure if this is a good idea, but can't exactly point to what might go wrong (except a slight breach of GC interface). Can you say what is the reason behind this?

I think this was supposed to be an optimisation. Not sure if it’s still required/effective.

> 
> Thanks,
> Roman

— Ron

> 
>> Hi Roman.
>> It means that oops in the object do not require barriers. For G1, that’s equivalent to whether or not the object
>> is in the young generation; for ZGC it means whether the object is in an allocating region.
>> It is used in the following way: continuation stacks are stored in special objects, known to the GCs, of type
>> StackChunk (see instanceStackChunkKlass.xxx). As long as that object does not require barriers, frames
>> can be copied into it (frozen) without parsing them and walking the oops. Once such an object starts requiring
>> barriers, frames can only be copied out of it (thawed), and freezing will allocate a new chunk.
>> This allows us to quickly freeze stack portions without parsing them and without changing the shape (location
>> of oops) when the object is in a region that requires barriers, so that the GC can assume that oop locations
>> are stable.
>> — Ron
>>> On 5 Feb 2021, at 19:17, Roman Kennke <rkennke at redhat.com> wrote:
>>> 
>>> Hello Loom devs,
>>> 
>>> I am currently trying to befriend Loom and Shenandoah GC. It's falling over this assert here:
>>> 
>>> https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/e8aeb3d34d9dee2f1f5c1ddc71a41adc991b5808/src/hotspot/share/runtime/continuation.cpp*L2650__;Iw!!GqivPVa7Brio!JdCI5e_oUvM0vddq0EEq1183zGwMWEwdly7p6oiE96kvHzFrUSpysBL5F8JJu1Migg$ 
>>> And thus I'm trying to figure out what requires_barriers() means in this context, what is the expected semantics of it.
>>> 
>>> In other GCs it seems to return false when the object is in a heap region that is not used for allocations:
>>> 
>>> E.g.:
>>> https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/e8aeb3d34d9dee2f1f5c1ddc71a41adc991b5808/src/hotspot/share/gc/g1/g1CollectedHeap.cpp*L2222__;Iw!!GqivPVa7Brio!JdCI5e_oUvM0vddq0EEq1183zGwMWEwdly7p6oiE96kvHzFrUSpysBL5F8KzaVrBHQ$ 
>>> or:
>>> 
>>> https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/e8aeb3d34d9dee2f1f5c1ddc71a41adc991b5808/src/hotspot/share/gc/z/zCollectedHeap.cpp*L124__;Iw!!GqivPVa7Brio!JdCI5e_oUvM0vddq0EEq1183zGwMWEwdly7p6oiE96kvHzFrUSpysBL5F8Kzt_H9_Q$ 
>>> However, the declaration here says something different, that it should return false when HeapAccess are equivalent to RawAccess:
>>> 
>>> https://urldefense.com/v3/__https://github.com/openjdk/loom/blob/e8aeb3d34d9dee2f1f5c1ddc71a41adc991b5808/src/hotspot/share/gc/shared/collectedHeap.hpp*L360__;Iw!!GqivPVa7Brio!JdCI5e_oUvM0vddq0EEq1183zGwMWEwdly7p6oiE96kvHzFrUSpysBL5F8I0DAefnA$ 
>>> In most GCs this would be the case when GC is not active. But that is not how it's implemented in other GCs.
>>> 
>>> Can you please shed some light what is the intention behind requires_barriers() and how it is used?
>>> 
>>> Thanks a lot!
>>> Roman
>>> 
>