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