RFR (S): 8035326: Assume non-NULL references in G1CollectedHeap::in_cset_fast_test

[Thomas Schatzl]

Hi,

On Thu, 2014-02-20 at 13:06 +0100, Mikael Gerdin wrote:
> Thomas,
> 
> On Thursday 20 February 2014 11.17.39 Thomas Schatzl wrote:
> > Hi Jon,
> > 
> >   thanks for the review.
> > 
> > On Wed, 2014-02-19 at 17:18 -0800, Jon Masamitsu wrote:
> > > Thomas,
> > > 
> > > Changes look good.
> > > 
> > > Is there really nothing you would put into the assert string?
> > > 
> > > http://cr.openjdk.java.net/~tschatzl/8035326/webrev/src/share/vm/gc_implem
> > > entation/g1/g1CollectedHeap.hpp.udiff.html
> > > 
> > > +    assert(_g1_committed.contains((HeapWord*) obj), "");
> > 
> > Fixed in http://cr.openjdk.java.net/~tschatzl/8035326/webrev.1 along
> > with other suggestions from Stefan.
> 
> in g1CollectedHeap.cpp
> 
> 4792   T heap_oop = oopDesc::load_heap_oop(p);
> 4793 
> 4794   // Filter out all NULL references up front avoiding checking this again
> 4795   // over and over.
> 4796   if (oopDesc::is_null(heap_oop)) {
> 4797     return;
> 4798   }
> 4799 
> 4800   oop obj = oopDesc::load_decode_heap_oop_not_null(p);
> 
> Why do you dereference p again after the null check? You could just have:
> oop obj = oopDesc::decode_heap_oop_not_null(heap_oop)

Thanks for catching this. Bug from splitting.

Also removed the comment as per suggestion from Stefan.

New webrev: http://cr.openjdk.java.net/~tschatzl/8035326/webrev.2

Thanks,
  Thomas