RFR: 8212656: Clean up heap filler insertion

[Per Liden]

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?

cheers,
Per

> 
> Roman
> 
>> While reviewing JDK-8211955, I suggested that the filler insertion code
>> in CollectedHeap should be broken out into a Fill class (to match the
>> somewhat related Copy class we already have). The filler insertion code
>> doesn't have to live in the already crowded CollectedHeap class, as it's
>> basically just a bunch of utility functions (just like the functions in
>> the Copy class).
>>
>> With this patch CollectedHeap calls Fill::initialize() in it's
>> constructor. This will set up the default behavior. Collectors with
>> special needs (G1, ZGC and Shenandoah) can then call one or more of the
>> Fill::set_*() functions to adjust filling behavior.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8212656
>> Webrev: http://cr.openjdk.java.net/~pliden/8212656/webrev.0
>>
>> Testing: Passed tier{3,5,6} on all Oracle supported platforms, running
>> additional tiers right now.
>>
>> /Per
>