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

[Per Liden]

Thanks Erik!

/Per

On 03/26/2018 09:10 AM, Erik Osterlund wrote:
> Hi Per,
> 
> Looks good.
> 
> Thanks,
> /Erik
> 
>> On 26 Mar 2018, at 08:40, Per Liden <per.liden at oracle.com> wrote:
>>
>> Hi again,
>>
>> It turned out that the Solaris Studio compiler didn't like structs with reference members without a user-defined constructor (all other compilers happily swallows this), so I had to add one.
>>
>> Updated webrev:
>>
>> Diff: http://cr.openjdk.java.net/~pliden/8199925/webrev.1vs2
>> Full: http://cr.openjdk.java.net/~pliden/8199925/webrev.2
>>
>> /Per
>>
>>> On 03/23/2018 11:57 AM, Per Liden wrote:
>>> 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
>>>>>>>
>>>>
>