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