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

[Per Liden]

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