GC interface update

[Per Liden]

On 2017-04-24 15:46, Roman Kennke wrote:
> Am 24.04.2017 um 08:37 schrieb Per Liden:
>> On 04/20/2017 02:29 PM, Roman Kennke wrote:
>>> Am 20.04.2017 um 14:01 schrieb Per Liden:
>>>> On 2017-04-20 12:05, Aleksey Shipilev wrote:
>>>>> On 04/20/2017 09:37 AM, Kirk Pepperdine wrote:
>>>>>>> Good stuff. However, one thing I'm not quite comfortable with is the
>>>>>>> introduction of the GC class (and its sub classes). I don't quite
>>>>>>> see the
>>>>>>> purpose of this interface split-up between GC and CollectedHeap. I
>>>>>>> view
>>>>>>> CollectedHeap as _the_ interface (but yes, it needs some love), and
>>>>>>> as a
>>>>>>> result I think the the functions you've exposed in the GC class
>>>>>>> actually
>>>>>>> belongs in CollectedHeap.
>>>>>>
>>>>>> I thought the name CollectedHeap implied the state of the heap
>>>>>> after the
>>>>>> collector has completed. What is the intent of CollectedHeap?
>>>>>
>>>>> No, CollectedHeap is the actual current GC interface. This is the
>>>>> entry point to
>>>>> GC as far as the rest of runtime is concerned, see e.g. CollectedHeap*
>>>>> Universe::create_heap(), etc. Implementing CollectedHeap,
>>>>> CollectorPolicy, and
>>>>> BarrierSet are the bare minimum required for GC implementation
>>>>> today. [1]
>>>>
>>>> Yep, and I'd like us to move towards tightening down the GC interface to
>>>> basically be cleaned up versions of CollectedHeap and BarrierSet.
>>>>
>>>> CollectorPolicy and some other things that class drags along, like
>>>> AdaptiveSizePolicy, are way too collector specific and I don't think
>>>> that should be exposed to the rest of the VM.
>>>
>>> Right, I totally agree with this.
>>>
>>> BTW, another reason for making a new GC interface class instead of
>>> further bloating CollectedHeap as the central interface was that there
>>> is way too much implementation stuff in CollectedHeap. Ideally, I'd like
>>> to have a true interface with no or only trivial implementations for the
>>> declared methods, and most importantly nothing that's only ever needed
>>> by the GC itself (and never called by the runtime). But as I said, I'm
>>> not against a serious refactoring and tightening-up of CollectedHeap
>>> instead.
>>
>> Yes, I'd like to keep CollectedHeap as the main interface, but I
>> completely agree that CollectedHeap currently contains too much
>> implementation stuff that we probably want to move out.
>
> Ok, I will revert that part of the change to use CollectedHeap as main
> interface then. It's no big deal, so far I only had one additional
> method for servicability support in the GC interface class anyway.

Ok, sounds good.

And regarding BarrierSet. As you know, Erik Österlund is working on 
overhauling BarrierSet and how barriers are used across the VM. He'll be 
sending out his current proposal later today.

>
> Would you also prefer keep 'management' of the heap in Universe too?
> I.e. Universe::create_heap() and Universe::heap() etc? Or do you see a
> benefit in moving it out like I did with gc_factory.cpp? The idea being
> that there's only one smallish place that knows about all the existing
> GC impls?

I'd like to keep Universe::heap() and create_heap(), but I'd like to 
move away from our current if-else if-else.. and instead have a more 
declarative way of saying which GC's are available. create_heap() would 
then just walk the list of available GC and ask if it's enabled and if 
so create an instance. I think we'd want to do something similar to (or 
even combine this with) what Erik is doing in his BarrierSet patch.

In general, to make it easier to review/test/integrate all these changes 
it would be good if we can have incremental patches, each addressing 
some specific/contained area.

cheers,
Per