RFR: JDK-8211955: GC abstraction for LAB reserve

[Roman Kennke]

Hi Per,

> On 10/11/2018 11:24 AM, Roman Kennke wrote:
> [...]
>>>> Ok, cool. Here's the updated webrevs:
>>>> Incremental:
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8211955/webrev.02.diff/
>>>> Full:
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8211955/webrev.02/
>>>>
>>>> Good now?
>>>
>>> I'm not sure I understand why we need yet another abstraction for this.
>>> I'm thinking the stuff you did in JDK-8211270 should be enough? We
>>> already have CollectedHeap::min_fill_size() to answer the question what
>>> the min filler size is, so adding a new function doesn't make sense to
>>> me. What am I missing?
> 
> I see now that I missed the second half of the JDK-8211270 mail thread.

;-)

>> min_fill_size() is static and cannot be overridden by GC. obj_size()
>> from JDK-8211270 could be used if we'd pass a specific object. I guess
>> we could make a better abstraction like what Erik suggested in the other
>> thread:
>>
>> virtual size_t cell_size(size_t obj_size) { return obj_size; }
> 
> I don't think we should introduced the concept of cell in this patch.
> That's a much larger concept/change that needs to be carefully thought
> out first.

Yeah. It should at least involve the allocation path. But let's drop it
for now.

> For example, it's not obvious to me if you consider the Brooks pointer
> to be an extension of the object header, and hence part of the object
> and included in obj->size(), of if it's a separate thing. If it's a
> separate thing, then a cell concept might prove useful. But if it's a
> separate thing, then JDK-8211270 now incorrectly made
> JvmtiEnv::GetObjectSize() and WB_GetObjectSize() return the cell size
> instead of the object size.
> 
> I assume you thought these things through? What's the story for
> Shenandoah here?

We never made the distinction between cell and object size. Practically
speaking, we treated the fwd pointer as part of the object, but made it
invisible to the rest of the runtime, i.e. it's GC specific. We wanted
to be honest about reporting the space that each object occupies in the
heap, hence the changes to WB and JVMTI.

>> and this could be called by JVMT and whitebox (JDK-8211270) with
>> obj->size(), and by min_fill_size() with the static minimum obj size,
>> and the GC would translate it to the cell size. The obj_size() from
>> JDK-8211270 would then be removed and we'd have a single abstraction for
>> this. Want me to make that change?
>>
>> Do you agree with the rest of this change? Because there's some
>> significant changes how TLAB and PLAB calculate their alloc reserves in
>> there too.
> 
> I think we need to think through the basics above first.
> 
> Generally, having PLAB call Universe::heap()->tlab_alloc_reserve() looks
> wrong to me, since a PLAB is not a TLAB. If we really want to share this
> code I think we should try to find some other way. Maybe just give
> tlab_alloc_reserve() a better name, but at the same time I'm not even
> sure this belongs in CollectedHeap.

We can change it to lab_alloc_reserve() instead. I think it should be
shared between TLAB and PLAB because the concept is the same thing, and
relies on the same properties: we need some reserve at the end in order
to be able to fill it completely, and with Shenandoah this is GC
specific. And since all the other code that deals with filling TLABs and
PLABs resides in CollectedHeap, this naturally belongs there too, right
next to CollectedHeap::fill_with_dummy_object(..) which is also used by
both TLAB and PLAB.

(BTW: some explanation why we care about PLAB at all: Shenandoah uses a
sort of GCLAB for the GC workers to evacuate objects from one region to
another. Java threads also use this for evacuations from write-barriers.
We did use TLAB for this in the past, but this was - and still is -
cumbersome because of the thight coupling of TLAB with Thread. For
example TLAB needs to know about its offset in Thread. That's why we
eventually switched to PLAB: this can be much easier used in
GCThreadLocalData structure. It fits our purpose very well. We could
have invented yet another LAB structure though...)

How do we want to go forward?

Roman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181011/2446e1e9/signature.asc>