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

[Erik Helin]

On 2014-11-17, Thomas Schatzl wrote:
> 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.

Thanks!

> > 
> > > 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.

I filed https://bugs.openjdk.java.net/browse/JDK-8065117.

Thanks,
Erik

> [...]
> > > 
> > >>>> 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
> 
>