RFR (S): 8201490: Improve concurrent mark keep alive closure performance

Thomas Schatzl thomas.schatzl at oracle.com
Fri Apr 13 08:18:17 UTC 2018


Hi Kim,

  thanks for your review.

On Thu, 2018-04-12 at 17:28 -0400, Kim Barrett wrote:
> > On Apr 12, 2018, at 3:57 PM, Thomas Schatzl <thomas.schatzl at oracle.
> > com> wrote:
> > 
> > Hi,
> > 
> >  can I have reviews for this change that improves the performance
> > of [...]
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8201490
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8201490/webrev
> > Testing:
> > hs-tier1-3
> > 
> > Thanks,
> >  Thomas
> 
> Looks good.  Just a couple minor comments.
> 
> -------------------------------------------------------------------
> ----------- 
> src/hotspot/share/gc/g1/g1ConcurrentMark.hpp
>  782   // Returns whether there has been a mark to the bitmap.
>  788   // Returns true if the reference caused a mark to be set in
> the next bitmap.
> 
> Consider making these the same, and something like:
> Returns true if a mark was added to the bitmap.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp
>  243   if (obj == NULL) {
>  244     return false;
>  245   }
>  246   return make_reference_grey(obj);
> 
> For me, something like the following would be easier to read:
> 
>   return (obj != NULL) && make_reference_grey(obj)
> 
> Up to you...
> 
> -------------------------------------------------------------------
> -----------
> 

All fixed.

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

Thanks,
  Thomas



More information about the hotspot-gc-dev mailing list