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

Roman Kennke rkennke at redhat.com
Tue Oct 17 21:21:58 UTC 2017


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




More information about the hotspot-gc-dev mailing list