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

[Zhengyu Gu]

Good to me.

-Zhengyu

On 10/17/2017 05:21 PM, Roman Kennke wrote:
> Hi Zhengyu,
> 
> thanks for reviewing!
> 
>> Hi Roman,
>>
>> Look good overall.
>>
>> 1) Probably make sense to factor out static methods from GC into 
>> separate GCFactory class.
> Right, this looks better.
> 
>> 2) Stylish:
>>   #ifndef SHARE_VM_GC_G1_G1_GC_HPP -> #ifndef SHARE_VM_GC_G1_G1GC_HPP
> Ok, right. I also removed the '_VM' parts to account for the changed 
> directory structure in mono-repo.
> 
> Differential:
> http://cr.openjdk.java.net/~rkennke/8189171/webrev.01.diff/ 
> <http://cr.openjdk.java.net/%7Erkennke/8189171/webrev.01.diff/>
> Full:
> http://cr.openjdk.java.net/~rkennke/8189171/webrev.01/ 
> <http://cr.openjdk.java.net/%7Erkennke/8189171/webrev.01/>
> 
> Better now?
> 
> Roman
> 
>>
>> Thanks,
>>
>> -Zhengyu
>>
>> On 10/12/2017 03:59 PM, 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.
>>>
>>> In order to have a cleaner GC interface, I propose to move all that 
>>> code into GC specific classes under their GC specific directories.
>>>
>>> Since at this point in time we haven't created the XXXHeap subclasses 
>>> yet (and cannot, before having set up all the flags and arguments), I 
>>> propose to create a GC interface of which we create an instance as 
>>> soon as we have selected a GC, and then do argument processing there.
>>>
>>> This 'GC' interface is, at this point, meant as a sort of GC factory 
>>> interface. With this patch, it will only do argument processing. 
>>> Later I plan to add heap creation to it (which is currently done in 
>>> universe.cpp, with INCLUDE_ALL_GCS stuff and all the if (UseBlahGC) 
>>> .. else if .. else branch chains. My current prototype in the jdk10 
>>> sandbox also provides servicabilty support classes and some more 
>>> stuff, but we should decide later where to put this.
>>>
>>> I broke out some code paths into gc_factory.cpp (i.e. not in gc.cpp). 
>>> Those are all the code paths that select GC, create the right 
>>> instances, etc, in other words, the code paths that need to know 
>>> about all GCs. Ideally, this should be the only file that needs to 
>>> know about all the GCs. (Suggestions for improvement are welcome of 
>>> course.)
>>>
>>> I tested this by running hotspot_gc jtreg tests, with normal 
>>> fastdebug and no-precompiled configurations. Everything passes fine.
>>>
>>> http://cr.openjdk.java.net/~rkennke/8189171/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Erkennke/8189171/webrev.00/>
>>>
>>> Opinions?
>>>
>>> Roman
>>>
>