RFR: 8212656: Clean up heap filler insertion

Roman Kennke rkennke at redhat.com
Thu Oct 18 14:55:24 UTC 2018


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.

Thanks,
Roman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20181018/1d9bdcfe/signature.asc>


More information about the hotspot-gc-dev mailing list