RFR (M): 8224741: Optimize the scan area during the Scan Heap Roots phase

Thomas Schatzl thomas.schatzl at oracle.com
Wed Jul 3 09:17:34 UTC 2019


Hi,

  thanks for your review.

On Tue, 2019-07-02 at 13:06 -0400, Kim Barrett wrote:
> > On Jul 2, 2019, at 6:40 AM, Thomas Schatzl <
> > thomas.schatzl at oracle.com> wrote:
> > Updated webrevs:
> > http://cr.openjdk.java.net/~tschatzl/8224741/webrev.0_to_1 (diff)
> > http://cr.openjdk.java.net/~tschatzl/8224741/webrev.1 (full)
> 
> Looks good.
> 
> A couple minor comment nits, and one (highly) optional suggestion.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/heapRegion.hpp 
> 
> Thanks for the "card" => "memregion" renamings.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/heapRegion.hpp 
>  296   // Returns the address after the last actually scanned or NULL
> if the area could
> 
> "the address after the last actually scanned" isn't really accurate.
> More accurate might be "the next unscanned address".
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/heapRegion.inline.hpp
>  296     // If obj is not an objArray and mr contains the start of
> the
>  297     // obj, then this could be an imprecise mark, and we need to
>  298     // process the entire object.
>  299     int size = obj->oop_iterate_size(cl);
>  300     return MAX2((HeapWord*)obj + size, mr.end());
> 
> Maybe mention here also that there aren't any objects after obj in
> the region.

Fixed.

> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/heapRegion.inline.hpp
>  300     return MAX2((HeapWord*)obj + size, mr.end());
> 
> obj + size could be in a region following this region.  I think
> rather
> than the above result, we could instead return the end of the region
> containing obj + size.  But I suspect this case also rarely occurs,
> and even then there's rarely going to be anything to be saved by
> that.

I kept the code as is. The result will most likely exceed the current
chunk to avoid any further processing anyway. This does not seem to be
worthwhile to optimize.

http://cr.openjdk.java.net/~tschatzl/8224741/webrev.1_to_2 (diff)
http://cr.openjdk.java.net/~tschatzl/8224741/webrev.2 (full)

(These are only comment adjustments as suggested here, so no testing)

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list