RFR (7xS): 8177044: Remove _scan_top from HeapRegion

[Thomas Schatzl]

Hi all,

  Erik had a look at these changes and had minor comments:

I.e. he asked about not removing one default value for a parameter in
the declaration of the constructor of G1UpdateRSOrPushRefOopClosure
(and do it later).

Also, one call to memset() is redundant and has been removed.

Here are the changes again:

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

Thanks,
  thomas

On Thu, 2017-05-11 at 16:18 +0200, Thomas Schatzl wrote:
> Hi Kim and Sangheon,
> 
> On Tue, 2017-05-09 at 11:12 -0700, sangheon wrote:
> > 
> > Hi Thomas,
> > 
> > On 05/05/2017 05:13 AM, Thomas Schatzl wrote:
> > > 
> > > 
> > > Hi all,
> > > 
> > >    recent reviews have made changes necessary to parts of the
> > > changeset
> > > chain.
> > > 
> > > Here is a list of links to updated webrevs. Since they have
> > > apparently
> > > not been reviewed yet, I simply overwrote the old webrevs.
> > > 
> > > JDK-8177044: Remove _scan_top from HeapRegion
> > > http://cr.openjdk.java.net/~tschatzl/8177044/webrev/
> > Looks good to me.
> > And agree to Kim about retaining the comment.
> > 
> > src/share/vm/gc/g1/g1RemSet.cpp
> >   765   if (scan_limit <= start) {
> >   766     // If the trimmed region is empty, the card must be
> > stale.
> >   767     return false;
> > 
>   thanks for your review.
> 
> For reference, here is the comment I intend to push:
> 
> --- old/src/share/vm/gc/g1/g1RemSet.cpp	2017-05-11
> 16:14:56.054517736 +0200
> +++ new/src/share/vm/gc/g1/g1RemSet.cpp	2017-05-11
> 16:14:55.951514554 +0200
> @@ -735,6 +735,7 @@
>  
>    HeapWord* scan_limit = _scan_state->scan_top(r->hrm_index());
>    if (scan_limit <= start) {
> +    // If the card starts above the area in the region containing
> objects to scan, skip it.
>      return false;
>    }
>  
> because the original comment is wrong now.
> 
> New Webrevs:
> http://cr.openjdk.java.net/~tschatzl/8177044/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8177044/webrev.1/ (full)
> 
> Thanks again for your reviews,
>   Thomas
>