RFR (7xS): 8178148: Log more detailed information about scan rs phase

Thomas Schatzl thomas.schatzl at oracle.com
Mon May 15 11:50:02 UTC 2017


Hi Sangheon,

  thanks for your review and sorry for the late reply; 8180048 got in
the way :)

On Tue, 2017-05-09 at 11:46 -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.
> > 
> > 
[...]
> > JDK-8178148: Log more detailed information about scan rs phase
> > http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
> Looks good.
> I have a few minor comments below:
> 
> ------------------------------------------
> src/share/vm/gc/g1/g1GCPhaseTimes.cpp
> copyright update.
> 
> -------------------------------------------
> src/share/vm/gc/g1/g1GCPhaseTimes.hpp
> copyright update.
> 
>    83   };
>    84  private:
> line separator between 83 and 84.
> 
> -------------------------------------------
> src/share/vm/gc/g1/g1ParScanThreadState.hpp
> copyright update.
> 
> -------------------------------------------
> src/share/vm/gc/g1/g1RemSet.hpp
>   215   size_t cards_scanned() { return _cards_scanned; }
>   216   size_t cards_claimed() { return _cards_claimed; }
>   217   size_t cards_skipped() { return _cards_skipped; }
> Adding const ?

All fixed.

> 
> -------------------------------------------
> src/share/vm/gc/g1/workerDataArray.inline.hpp
>    65   assert(index < MaxThreadWorkItems, "Tried to access thread
> work 
> item %u max %u", index, MaxThreadWorkItems);
> Assert message can be improved with describing more explicitly like 
> which one is maximum value etc..
> e.g. "... work item %u (max=%u)
> 
>    71   assert(index < MaxThreadWorkItems, "Tried to access thread
> work 
> item %u max %u", index, MaxThreadWorkItems);
> Same as above.
> 

The max value is printed as your example actually shows. But I added
the parentheses.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8178148/webrev.0_to_1 (diff)
http://cr.openjdk.java.net/~tschatzl/8178148/webrev.1 (full)

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list