RFR (M): 8159422: Very high Concurrent Mark mark stack contention

[Thomas Schatzl]

Hi Kim,

On Wed, 2016-09-14 at 11:42 -0400, Kim Barrett wrote:
> > 
> > On Sep 14, 2016, at 8:54 AM, Thomas Schatzl <thomas.schatzl at oracle.
> > com> wrote:
> > 
> > Hi Kim,
> > 
> > On Tue, 2016-09-13 at 12:19 -0400, Kim Barrett wrote:
> > > 
> > > > 
> > > > 
> > > > On Sep 13, 2016, at 6:15 AM, Thomas Schatzl <thomas.schatzl at ora
> > > > cle.
> > > > com> wrote:
> > > > http://cr.openjdk.java.net/~tschatzl/8159422/webrev.2_to_3/
> > > > (diff)
> > > > http://cr.openjdk.java.net/~tschatzl/8159422/webrev.3/ (full)
> > > ---------------------------------------------------------------
> > > ----
> > > ----------- 
> > > src/share/vm/gc/g1/g1ConcurrentMark.cpp
> > >  242 G1CMMarkStack::OopChunk*
> > > G1CMMarkStack::remove_chunk_from_chunk_list() {
> > >  243   MutexLockerEx x(MarkStackChunkList_lock,
> > > Mutex::_no_safepoint_check_flag);
> > >  244   _chunks_in_chunk_list--;
> > >  245   return remove_chunk_from_list(&_chunk_list);
> > >  246 }
> > > 
> > > Decrement of _chunks_in_chunk_list needs to be conditional on
> > > result
> > > of remove_chunk_from_list, only decrementing if got a chunk.
> > > 
> >   it did not fail with the latest jprt run (I am basically always
> > doing
> > jprt runs for all changes I send out) because this method (and all
> > these new shorthand methods) have not actually been called.
> I missed that too!
> 
> > 
> > > 
> > > src/share/vm/gc/g1/g1ConcurrentMark.hpp
> > > I think some of the padding in G1CMMarkStack isn't useful with
> > > the
> > > lock-based approach.  I'm assuming you've left it because you
> > > plan to
> > > get back to the lock-free approach later.  Is there a CR for
> > > that?
> > > 
> > I can only see that the padding for
> > _chunks_in_chunk_list/_chunk_list
> > might not be useful as they are always accessed together.
> > I removed it.
> OK
> 
> > 
> > The new CR is JDK-8165848 (also referenced by this CR).
> OK
> 
> > 
> > Changes at
> > http://cr.openjdk.java.net/~tschatzl/8159422/webrev.3_to_4/ (diff)
> > http://cr.openjdk.java.net/~tschatzl/8159422/webrev.4/ (full)
> Looks good.
> 

  thanks a lot for your continued reviews.

Thanks,
  Thomas