RFR: 8144072: G1ParScanThreadState::update_rs does not need to call is_in_reserved

Erik Helin erik.helin at oracle.com
Thu Nov 26 13:27:59 UTC 2015


On 2015-11-26, Thomas Schatzl wrote:
> Hi,
> 
> On Wed, 2015-11-25 at 19:17 +0100, Erik Helin wrote:
> > Hi all,
> > 
> > based on the patch "8144067: Pass obj directly to
> > G1ParScanThreadState::update_rs", here is another small optimization
> > that can utilize that G1ParScanThreadState::update_rs now has access to
> > both the field and the obj. Instead of calling from->is_in_reserved(o),
> > which is defined as
> > 
> >   bool is_in_reserved(const void* p) {
> >     return _bottom <= p && p < _end;
> >   }
> > 
> > we can do the same things as the write barrier for G1:
> > 
> >   bool is_in_same_region(T* p, oop o) {
> >     return ((p ^ o) >> Log2HeapRegionSize) == 0;
> >   }
> 
> I think the function should assert that both p and o are non-NULL. In
> the current uses this is true. I would not need to re-review for this
> change.

Sure, I add the asserts before I push.

> > 
> > which requires no loads at all.
> 
> Well, Log2HeapRegionSize.

Right, sorry, I forgot about that one.

Thanks fo reviewing!
Erik

> > This patch adds the new static function HeapRegion::is_in_same_region
> > and patches G1ParScanThreadState::update_rs to make use of it.
> > 
> > Thanks to Thomas for pointing this optimization out.
> > 
> > Bug:
> > https://bugs.openjdk.java.net/browse/JDK-8144072
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~ehelin/8144072/webrev.00/
> > 
> > Please note this patch is based upon the patch for 8144067.
> > 
> > Testing:
> > - GC test suite with full verification.
> > 
> 
> Good.
> 
> Thanks,
>   Thomas
> 
> 


More information about the hotspot-gc-dev mailing list