RFR: JDK-8211955: GC abstraction for LAB reserve

[Per Liden]

Hi Roman,

On 10/15/2018 03:27 PM, Roman Kennke wrote:
> 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.

Sure. I still have at least one bug to sort out in my patch, and today 
I've been derailed by some higher priority stuff. So, go ahead and I'll 
follow up with my patch later.

cheers,
Per

> 
> 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
>