GC interface update

[Roman Kennke]

Am 25.04.2017 um 14:05 schrieb 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.

Working on it in the sandbox repo.

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

Great!

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

Sounds very reasonable to me.

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

Yep. Hence my attempt to break up the big 'GC interface' thingy into
smaller chunks today :-)

Roman

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