RFR: JDK-8211955: GC abstraction for LAB reserve

[Per Liden]

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