Add release barriers when allocating objects with concurrent collection

[Stuart Monteith]

Hi,
	Erik got in there before me. To add to what he said, store_klass is called in one place, 
cpu/aarch64/templateTable_aarch64.cpp:TemplateTable::_new(). Both _new() and C1_MacroAssembler::initialize_object emit a 
StoreStore barrier at the end of the constructor, as that is all they need for ordering on the TLAB and Eden.

 From reading the shared code, there is a protocol that GC should leave objects along if their klass field is null, 
which is what the ordering is applied to in this case. I can't find evidence that this is observed by the GC algorithms.

As for fields being initialized, and shared between classes, I thought that was well defined by the JVM spec - it would 
take a Thread to deliberately share it in that instance.

BR,
	Stuart

On 28/08/2020 09:09, Erik Österlund wrote:
> Hi Andrew,
> 
> This is an excellent question. In fact, perhaps this should be better documented in the code.
> 
> You should not need any fencing in the fast path, and the race indeed only happens for the
> slow path.
> 
> If you allocate an object in a TLAB, then it will be allocated in memory that is not concurrently
> visited by any GC in HotSpot (eden for STW collectors, allocating pages for ZGC, something similar
> for Shenandoah I guess). And they are simply not traversed concurrently by the GC.
> 
> So to the best of my knowledge, this code is really there only for huge allocations.
> 
> Huge allocations may be done in fast paths when CollectedHeap::supports_inline_contig_alloc() is true.
> But it is not true for the collectors that poke said objects concurrently. Therefore, such objects
> will instead be allocated with a slow path call, which will perform the right release_store dance
> in the runtime, for such huge objects.
> 
> These huge objects are not necessarily part of young gen, and may be traversed concurrently.
> 
> So as other objects have at least the header initialized in young gen or equivalent (allocating) memory,
> they should not need any release_store when initializing the Klass header, to satisfy the GC.
> 
> However, I'm not sure what happens if you share objects that have not finished running their constructor
> to e.g. a static field, and another Java thread racingly reads it and tries to make sense out of it.
> Feels like that might be a bit awkward...
> 
> Thanks,
> /Erik
> 
> On 2020-08-27 11:59, Andrew Haley wrote:
>> 8165808 (Add release barriers when allocating objects with concurrent
>> collection) added release barriers to object allocation paths. But it
>> only added them in shared code, not in the fast-path code.
>>
>> I find a comment I added to AArch64 in 2013:
>>
>>   void MacroAssembler::store_klass(Register dst, Register src) {
>> +  // FIXME: Should this be a store release?  concurrent gcs assumes
>> +  // klass length is valid if klass field is not null.
>>
>> PPC doesn't have release barriers here either:
>>
>> void MacroAssembler::store_klass(Register dst_oop, Register klass, Register ck) {
>>    if (UseCompressedClassPointers) {
>>      encode_klass_not_null(ck, klass);
>>      stw(ck, oopDesc::klass_offset_in_bytes(), dst_oop);
>>    } else {
>>      std(klass, oopDesc::klass_offset_in_bytes(), dst_oop);
>>    }
>> }
>>
>> So what's up? What should we be doing here? Surely if the slow path
>> code needs the barriers, the fast path does too.
>>
>