RFR: 8199925: Break out GC selection logic from GCArguments to GCSelector (original) (raw)

Per Liden per.liden at oracle.com
Fri Mar 23 10:57:03 UTC 2018

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::isgc* 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 (WBCurrentGC, WBAllSupportedGC, WBGCSelectedByErgo), 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