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

Kim Barrett kim.barrett at oracle.com
Wed Oct 18 02:06:23 UTC 2017


> 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

Note that the RFR message indicates Parallel derives its heap from
GCH, but it doesn't.  Not that it invalidates the reasons for making
this change.

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

------------------------------------------------------------------------------




More information about the hotspot-gc-dev mailing list