RFR: 8064721: The card tables only ever need two covering regions

Thomas Schatzl thomas.schatzl at oracle.com
Mon Nov 17 12:13:51 UTC 2014


Hi,

On Fri, 2014-11-14 at 16:01 -0500, Kim Barrett wrote:
> On Nov 14, 2014, at 8:48 AM, Erik Helin <erik.helin at oracle.com> wrote:
> > 
> > new webrevs available at:
> > - full:
> >  http://cr.openjdk.java.net/~ehelin/8064721/webrev.01/
> > - incremental:
> >  http://cr.openjdk.java.net/~ehelin/8064721/webrev.00-01/
> 
> New webrev looks good.

Looks good to me too.

> 
> > See comments inline.
> > 
> > On 2014-11-13 20:33, Kim Barrett wrote:
> >>>   57   static const int _max_covered_regions = 2;
> >> […] But there is also the
> >> question of whether this is any different from the maximum number of
> >> generations, […]
> > 
> > Ah ok, now I see. Yes, once the generations cleanup has been pushed, we should use GenCollectedHeap::_max_gens.
> 
> Do we need a bug for this?

Please file a bug for that that depends on the generations cleanup.

[...]
> > 
> >>>> src/share/vm/memory/modRefBarrierSet.hpp
> >>>>   40   ModRefBarrierSet() : BarrierSet() { _kind = BarrierSet::ModRef; }
> >> 
> >> But the initialization of _kind really should be in the member
> >> initialization list, and since you are touching the code anyway…
> > 
> > _kind is actually a member of BarrierSet, not ModRefBarrierSet, so it can't be set in member initialization list of a ModRefBarrierSet constructor. The proper way to clean this up would be to have the BarrierSet constructor take a parameter of type BarrierSet::Name to set _kind. In order to keep this patch focused on only cleaning up max_covered_regions, I reverted the patch to use the implicit call to the BarrierSet constructor in modRefBarrierSet.cpp.
> 
> Sorry I missed that.  Yes, the proper fix is to change the BarrierSet constructor.  And get rid of the Uninit tag in the BarrierSet::Name enum.  Assuming there is never a genuine need for a default constructed BarrierSet, which seems likely to me.  (ctor and dtor should also be protected.)
> https://bugs.openjdk.java.net/browse/JDK-8064947
> 

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list