RFR: 8199925: Break out GC selection logic from GCArguments to GCSelector

[Per Liden]

Thanks Roman and Erik for reviewing!

/Per

On 03/21/2018 04:07 PM, Roman Kennke wrote:
> Looks good! Thank you!
> 
> Am 21. März 2018 14:11:52 MEZ schrieb Per Liden <per.liden at oracle.com>:
>> Hi,
>>
>> Here's an updated webrev. I renamed GCSelector to GCConfig and it's now
>>
>> also the goto place for getting hold of the GCArguments instance for
>> the
>> currently selected GC.
>>
>> http://cr.openjdk.java.net/~pliden/8199925/webrev.1
>>
>> Btw, this is built on top of JDK-8199850 (Move parsing of VerifyGCType
>> to G1) and JDK-8199918 (Shorten names of CollectedHeap::Name members),
>> and the GCConfig::is_gc_* functions will eventually be used by
>> JDK-8199927 (Make WhiteBox more GC agnostic).
>>
>> /Per
>>
>> On 03/21/2018 11:33 AM, Per Liden wrote:
>>> Hi Roman,
>>>
>>> On 03/21/2018 10:15 AM, Roman Kennke wrote:
>>>> Am 21.03.2018 um 09:12 schrieb Per Liden:
>>>>> In an attempt to make WhiteBox more GC agnostic, and in turn make
>> it
>>>>> easier to plugin new GC without touching a lot of non-GC code, this
>>>>> patch breaks out the GC selection logic from GCArguments to
>> GCSelector.
>>>>> There are two reasons why I think this makes sense:
>>>>> 1) The GC selection logic is self-contained and fairly unrelated to
>> the
>>>>> rest of the flags processing done by GCArguments and its
>> GC-specific
>>>>> children.
>>>>> 2) Parts of the GC selection logic is needed by WhiteBox to answer
>>>>> questions about which GC are supported, selected, etc.
>>>>>
>>>>> A follow up patch (JDK-8199927) will change WhiteBox (WB_CurrentGC,
>>>>> WB_AllSupportedGC, WB_GCSelectedByErgo), to use the GCSelector.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199925
>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8199925/webrev.0
>>>>>
>>>>> /Per
>>>>
>>>> My original proposal for the GCArguments did have a separate
>>>> GCFactory. :-)
>>>>
>>>> In generally I like your change.
>>>>
>>>> I'd probably go even further and let GCSelector initialize and
>> return an
>>>> instance of GCArguments. This way we wouldn't end up with two places
>>>> that have knowledge of all possible GCs, but would only have one
>> place
>>>> (GCSelector) where one had to hook up for adding a new GC.
>>>>
>>>> What do you think?
>>>
>>> My intention here was that GCSelector would be a pure translator of
>>> Use*GC flags to CollectedHeap::Name, i.e. a small and well defined
>>> component, which knows nothing about GCArguments. The purpose of this
>>
>>> was to clean out the WhiteBox stuff.
>>>
>>> On the other hand, I see what you mean, and too would like to reach a
>>
>>> state where we have one place in the code that "knows about all the
>> GCs".
>>>
>>> Let me think about it some more and maybe prototype an alternative,
>> and
>>> we'll see if that gets us to a better place.
>>>
>>> /Per
>>>
>>>>
>>>> Roman
>>>>
>