RFR: 8189171: Move GC argument processing into GC specific classes

[Roman Kennke]

Hi Erik,

Yes that is fine.

Keep in mind that I'm planning to put heap creation into that class too.

I was thinking maybe to simply reverse the current naming: name the 
all-static 'factory factory' 'GC', and thus call GC::initialize() and 
GC::factory() or such, and the main interface GCFactory or 
CollectedHeapFactory. What do you think?

Roman


> I would like to keep the CollectedHeap as the facade interface to the 
> GC, like it is today, instead of having a new GC class making it two 
> facade interfaces.
> Of course, the CollectedHeap may only be used as a facade after it has 
> been created. And now we are dealing with code before it was created 
> during the bootstrapping process.
>
> So what I would like to have here is a class specifically for that, 
> rather than another GC facade interface. I think this is mostly an 
> exercise of finding a better name for the class you call "GC". Now I 
> am not an expert in coming up with good names myself, but some name 
> that indicates this is being used for flag processing would be good. 
> Perhaps GCArgumentProcessor. The benefit of calling it something like 
> that is that if a GC requires argument processing later in the 
> bootstrapping, possibly after CollectedHeap has been created, it can 
> still be used for this purpose without having to feel weird about it.
>
> How would you feel about that? Does it seem to make sense?
>
> Thanks,
> /Erik
>
> On 2017-10-19 21:14, Roman Kennke wrote:
>> Am 13.10.2017 um 03:20 schrieb David Holmes:
>>> Hi Roman,
>>>
>>> Not a review as GC folk need to do that.
>>>
>>> On 13/10/2017 5:59 AM, Roman Kennke wrote:
>>>> I'm posting it to both hotspot-runtime-dev and hotspot-gc-dev 
>>>> because it touches both areas (i.e. the GC interface).
>>>>
>>>> Currently, all GC related argument processing is done in 
>>>> arguments.cpp, littering it with #ifdef INCLUDE_ALL_GCS and all 
>>>> sorts of GC specific methods etc.
>>>
>>> From a runtime perspective I like all the GC specific ifdefs and 
>>> settings moved out-of-line of the main argument processing.
>>>
>>> From a refactoring perspective I noticed that set_object_alignment() 
>>> no longer calls set_cms_values(). I presume setting it elsewhere is 
>>> okay?
>> I totally forgot to reply to this.
>>
>> What's important is that the CMS alignment values are set after 
>> set_object_alignment() figured out the object alignment. The patch 
>> moves that a little further down the road to the beginning of its GC 
>> specific argument processing, but from GC perspective should be the 
>> same. I looked thoroughly through the involved code paths and cannot 
>> see what could go wrong.
>>
>> I need one more review. GC folks? The current webrev is:
>> http://cr.openjdk.java.net/~rkennke/8189171/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Erkennke/8189171/webrev.01/>
>>
>> Thanks,
>> Roman
>>
>