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

[Roman Kennke]

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

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.