RFR (M): 8077144: Concurrent mark initialization takes too long

[Thomas Schatzl]

Hi,

On Sat, 2016-03-26 at 13:06 -0400, Kim Barrett wrote:
> > On Mar 25, 2016, at 9:38 PM, Kim Barrett <kim.barrett at oracle.com>
> > wrote:
> > 
> > > On Mar 15, 2016, at 6:12 PM, Thomas Schatzl <
> > > thomas.schatzl at oracle.com> wrote:
> > > 
> > > Hi Mikael,
> > > 
> > > updated webrev at
> > > http://cr.openjdk.java.net/~tschatzl/8077144/webrev.3/ (full)
> > > http://cr.openjdk.java.net/~tschatzl/8077144/webrev.2_to_3/ (diff
> > > )
> > > 
> > > which implements the suggested changes.
> > 
> > -------------------------------------------------------------------
> > -----------
> > src/share/vm/gc/g1/g1ConcurrentMark.cpp
> > 1130 class G1LiveDataHelper VALUE_OBJ_CLASS_SPEC {
> > ...
> > 1142   inline void set_card_bitmap_range(BitMap* bm,
> > 
> > This uses non-parallel set_bit and set_range. Parallel callers are
> > operating on distinct heap regions. If that isn't guaranteed, then
> > two
> > parallel tasks could try to modify bits in the same BitMap word
> > simultaneously. We're OK so long as G1HeapRegionSize is a multiple
> > of
> > 
> >  BitsPerWord * (1 << CardTableModRefBs::card_shift)
> > 
> > (which is 2^15 with 512 byte cards on a 64bit machine), but
> > G1HeapRegionSize is a product option with a relatively
> > unconstrained
> > value (only bounded between 1M and 32M, no alignment constraint).

The latter is wrong. G1 has alignment constraints, all regions must be
aligned to their size.

This is enforced in the heap initialization, look for callers to
G1CollectorPolicy::heap_alignment() and how it is calculated.

> > Oh, but, it looks like HeapRegion::setup_heap_region_size ensures
> > the region size is a power of 2 within the min/max bounds, rounding
> > up an arbitrary command line value if necessary.
> > 
> > So we're OK, though it required some searching to convince myself 
> > of that.  An assertion somewhere in G1LiveDataHelper might be
> > appropriate, since it doesn't ever use par_xxx bitmap functions,
> > unlike the old code, which selected between them based on an 
> > argument that no longer exists.
> 
> Well, we’re OK size-wise.  I’m pretty sure we’re OK alignment-wise 
> too, but I’ve not yet (re)found the code that would convince me of 
> that. Checking that would be part of the requested assertion.

Already improved that check in JDK-8151386. Please look there.

Thanks,
  Thomas