RFR: JDK-8211955: GC abstraction for LAB reserve

[Roman Kennke]

Hi Per,

what do you think about putting this proposed change in jdk/jdk now, and
work on the improvements later / on top of it? It does provide some
reasonable consolidation I think, which seems worth as-is.

Thanks,
Roman


> Hi Roman,
> 
> On 10/11/2018 02:41 PM, Roman Kennke wrote:
> [...]
>> Those are the only functions where this matters. In-fact, the LAB
>> reserve abstraction is the last one I have lined up. The remaining stuff
>> that Shenandoah needs is a bunch of extra utility classes and C2 changes.
> 
> Thanks for clarifying. That makes me much less worried.
> 
> For C2, the strategy we had and the requirement we put on our selves
> when integrating ZGC was basically this:
> 
> We really didn't want to screw things up in C2. If things went south, we
> wanted to just be able to disable building of ZGC and C2 would go back
> to do _exactly_ what it did before we merged the ZGC patch. As you maybe
> remember, we _almost_ got there. We had a very small set of places in C2
> where we did change/add code, which was _not_ disabled when disabling
> ZGC. However, it was easy to prove that that code was safe/correct. This
> strategy turned out to work well, and it forced us to think hard about
> having abstractions in the right places, move as much code as possible
> into the BarrierSet, etc.
> 
> I'd recommend you do the same for Shenandoah, that way the pressure on
> you (in case something breaks and results instability or a long bug
> trail) will be reduced a lot. If shit hits the fan, you can always
> disable building of Shenandoah until the problems are sorted out.
> 
> [...]
>>>>> 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.
>>>
>>> Maybe a stupid question, but why do you even need a Brooks pointer in
>>> your filler objects? Is not having that maybe a solution here?
>>
>> Filler objects are objects. Laying them out differently would make
>> parsing the heap impossible.
> 
> Check.
> 
> I thought some more about this. How about we rip out the filler-stuff
> from CollectedHeap, and put it in a Fill utility class. You could think
> of Fill as a cosine to the "class Copy" we have today. Something like:
> 
> class Fill : public AllStatic {
> private:
>   static int _min_size;
> 
> public:
>   static void set_min_size(int min_size) {
>     _min_size = min_size;
>   }
> 
>   static int min_size() {
>     return _min_size;
>   }
> 
>   static int min_reserve_size() {
>     return _min_size > MinObjAlignment ? _min_size : 0;
>   }
> 
>   static void with_objects(...);
>   static void with_object(...);
>   ...
> };
> 
> Fill::_min_size is initialized to oopDesc::header_size() by default. A
> collector that wants something different calls Fill::set_min_size(...)
> during start up. TLAB/PLAB would call Fill::min_reserve_size().
> 
> In my mind, I think this would put things where they belong. And as a
> bonus we would cut out some cruft from CollectedHeap. What do you think?
> I started doing a patch for this to see if it seems like a good
> approach. I hope to have something to show later today.
> 
> cheers,
> Per

-------------- 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/20181015/05ef5fca/signature.asc>