RFR: 8199925: Break out GC selection logic from GCArguments to GCSelector
Per Liden
per.liden at oracle.com
Mon Mar 26 06:40:12 UTC 2018
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
>>>>>
>>
More information about the hotspot-gc-dev
mailing list