RFR (M): 8189737: Make HeapRegion not derive from Space

Kim Barrett kim.barrett at oracle.com
Thu Oct 31 22:12:20 UTC 2019


> On Oct 31, 2019, at 9:43 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I get reviews for this refactoring that removes the inheritance of HeapRegion from Space?
> 
> Since JDK10 we did not use much of the shared code in G1, so apart from inheriting a few trivial members (bottom, top, compaction_top) there is not much gain in inheriting from (Contiguous-)Space, except adding quite a few unused members and lots of legacy code.
> 
> In JDK10 we already considered removing this inheritance, but never got around until now :)
> 
> There will be a follow-up JDK-8233306 that cleans up the code a bit (sorting members and methods), but to keep this a bit more easily reviewable, the change is as it is.
> 
> The change is smaller than webrev indicates, for some reason the single-line include change in test_g1HeapVerifier.cpp caused it to be included as a "new" file. There is also a lot of one-line #include-wrangling.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8189737
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8189737/webrev/
> Testing:
> hs-tier-1-5
> 
> Thanks,
>  Thomas

It's a little unfortunate that you needed to touch the #includes in a
couple of cms files.  Looks like it should be an easy merge for
whichever of you or Leo goes second though.

------------------------------------------------------------------------------
 102 inline HeapWord* HeapRegion::par_allocate(size_t min_word_size,
 103                                                  size_t desired_word_size,
 104                                                  size_t* actual_size) {

Parameter list indentation needs fixing.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/heapRegion.hpp

There's a big comment block about BlockOffsetTable divergence, time
stamps, &etc in front of G1ContiguousSpace that seems to have simply
disappeared. I take it this was leftover commentary that should have
been removed with JDK-8199326 and maybe others?

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

Looks good.

I don't need a new webrev for the parameter list indentation fix.




More information about the hotspot-gc-dev mailing list