RFR: 8183542: Factor out serial GC specific code from GenCollectedHeap into its own subclass

[Roman Kennke]

Hi Kim,

thanks for reviewing!

>> On Oct 13, 2017, at 10:25 AM, Roman Kennke <rkennke at redhat.com> wrote:
>>
>> This gives the serial GC its own (Gen)CollectedHeap subclass. There is not much serial specific code in GCH, but it seems odd to report it as 'serial' while its subclasses override it as CMS or Parallel.
>>
>> Tested successfully with hotspot_gc.
>>
>> http://cr.openjdk.java.net/~rkennke/8183542/webrev.00/ <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.00/>
>>
>> Opinions? Ok to go in?
>>
>> Thanks, Roman
> I have a deeply ingrained aversion to classes which are used as both
> base classes and as concrete (directly instantiated) classes.  So I
> approve of this change.  Just a few minor and style comments.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/collectedHeap.hpp
>    82 // CollectedHeap
>    83 //   SerialHeap
>    84 //   G1CollectedHeap
>    85 //   ParallelScavengeHeap
>    86 //   CMSHeap
>
> Isn't this really
>
> // CollectedHeap
> //   GenCollectedHeap
> //     SerialHeap
> //     CMSHeap
> //   G1CollectedHeap
> //   ParallelScavengeHeap
Ok, I fixed that.
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/genCollectedHeap.hpp
>   164   CollectorPolicy* collector_policy() const { return gen_policy(); }
>   167   AdaptiveSizePolicy* size_policy() {
>
> Why were the virtual qualifiers removed from these definitions?
> Hotspot style seems to tend toward being explicit about virtual in
> derived classes too (though it's far from universal). (Maybe someday
> we'll be able to use C++11 override specifiers.) These are declared in
> CollectedHeap.
>
> Hm, this seems to have been done for a whole lot more functions.  And
> yet, print_gc_threads_on and gc_threads_do weren't changed that way,
> even though immediately bracketed by declarations that were changed.
>
> I'm going to say no, don't do that.
>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/serial/serialHeap.hpp
>
> As mentioned above, common style is (I think) to be explicit about
> virtual in override declarations.
>
> ------------------------------------------------------------------------------
>
Ok, I added back all the virtual qualifiers.

Differential webrev:
http://cr.openjdk.java.net/~rkennke/8183542/webrev.01.diff/ 
<http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01.diff/>

Full webrev:
http://cr.openjdk.java.net/~rkennke/8183542/webrev.01/ 
<http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01/>

Better now?

Thanks, Roman