RFR: 8212656: Clean up heap filler insertion

[Per Lidén]

> On 18 Oct 2018, at 16:55, Roman Kennke <rkennke at redhat.com> wrote:
> 
> Hi Per,
> 
>> Hi Roman,
>> 
>>> On 10/18/2018 03:57 PM, Roman Kennke wrote:
>>> Hi Per,
>>> 
>>> I'll try to weave that into Shenandoah and see how that goes :-)
>>> 
>>> One thing that I don't see how it could work is how we could do extra
>>> initialization for the fwd ptr? We can currently do it by overriding
>>> fill_with_dummy_object(..) like this:
>>> 
>>> void ShenandoahHeap::fill_with_dummy_object(HeapWord* start, HeapWord*
>>> end, bool zap) {
>>>    HeapWord* obj = post_allocation_setup(start); // Bumps start by 1 word
>>>    CollectedHeap::fill_with_object(obj, end);
>>> }
>>> 
>>> This seems to have gone with your patch.
>> 
>> Ok, so if we do something like this:
>> 
>> --- a/src/hotspot/share/gc/shared/fill.cpp
>> +++ b/src/hotspot/share/gc/shared/fill.cpp
>> @@ -76,14 +76,14 @@
>> 
>>  void Fill::fill_with_object(HeapWord* start) {
>>    const size_t size = object_header_size();
>> -  ObjAllocator allocator(SystemDictionary::Object_klass(), size);
>> -  allocator.initialize(start);
>> +  ObjAllocator allocator(SystemDictionary::Object_klass(), size -
>> _extra_header_size);
>> +  allocator.initialize(start + _extra_header_size);
>>  }
>> 
>>  void Fill::fill_with_array(HeapWord* start, size_t size) {
>> -  const size_t length = array_length(size);
>> -  ObjArrayAllocator allocator(Universe::intArrayKlassObj(), size,
>> (int)length, false /* do_zero */);
>> -  allocator.initialize(start);
>> +  const size_t length = array_length(size - _extra_header_size);
>> +  ObjArrayAllocator allocator(Universe::intArrayKlassObj(), size -
>> _extra_header_size, (int)length, false /* do_zero */);
>> +  allocator.initialize(start + _extra_header_size);
>> 
>>    if (ZapFillerObjects) {
>>      HeapWord* const payload_start = start + array_header_size();
>> 
>> Then the object will be initialized in the correct position. You will
>> not have the forwarding pointer initialized (we only made space for it
>> to keep the layout intact), but I suspect that should not be needed for
>> this purpose. The only reason this object exists is to make the heap
>> parsable, so you should only be reading the klass pointer anyway, right?
> 
> Or, instead of hardwiring to ObjAllocator and ArrayAllocator, call
> CH::obj_allocate() and CH::array_allocate() which will be overridden by
> our own stuff, and should do the right thing? I'll try this approach in
> a bit.

I’m afraid that will call allocator.allocate() rather than just allocator.initialize().

/Per

> 
> Thanks,
> Roman
>