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