RFR (M): 8226913: Scale cards per chunk used during heap root scanning with region

[Kim Barrett]

> On Jul 10, 2019, at 11:48 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I have reviews for this change that scales the amount of cards
> processed per chunk depending on heap/heap region size to decrease some
> unnecessary overhead when clearing it on larger heaps.
> 
> It is basically the answer to this comment in the code:
>  96   // Random power of two number of cards we want to claim per
> thread. This corresponds
>  97   // to a 64k of memory work chunk area for every thread.
>  98   // We use the same claim size as Parallel GC. No particular
> measurements have been
>  99   // performed to determine an optimal number.
> 
> I.e. the chosen number simply seemed suboptimal, and should actually be
> determined adaptively.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8226913
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8226913/webrev
> Testing:
> hs-tier1-5, manually checking that correct chunk sizes are returned for
> all possible region sizes
> 
> Thanks,
>  Thomas

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1RemSet.cpp
  98   // size we limit the total number of chunks to limit memory usage and maintenance

I think there should be a sentence break in "size we".

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1RemSet.cpp
 100   // Testing showed that 8 for 1M/2M region, 16 for 4M/8M regions, 32 for 16/32M regions
 101   // seems to be such a good trade-off.
...
 105     // The original formula is y = 0.4 * x - 5. The below formula is tweaked so

I'm not sure how that formula was derived (some linear fit?), but

  1u << ((log_region_size / 2) - 7) 

seems to give equivalent results for the given range and seems (to me)
more "obviously" based on the given numbers.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1RemSet.cpp
 103     assert(log_region_size >= 20 && log_region_size <= 25,
 104            "expected value in [20,25], but got %u", log_region_size);

This is hard-coded to the current region size range limits, and
doesn't seem to be related to any requirements of the calcuation being
done. Any future expansion of the permitted region size range would
fail here for no immediately obvious reason. The only reason to choose
those bounds here seems to be that those are what's actually been
tested, so as to not make any assumptions about what might be good
outside that range.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1RemSet.cpp
 370     return result * (HeapRegion::CardsPerRegion / get_chunks_per_region(HeapRegion::LogOfHRGrainBytes));

Instead of get_chunks_per_region(...), why not _scan_chunks_per_region?

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