RFR (S): 8037344: Use the "next" field to iterate over fine remembered instead of using the hash table

Mikael Gerdin mikael.gerdin at oracle.com
Wed Apr 9 11:46:34 UTC 2014


Hi Thomas,

On Wednesday 09 April 2014 12.02.45 Thomas Schatzl wrote:
> Hi all,
> 
>   I have been notified that the webrev does not apply to the jdk9/hs-gc
> repository any more.
> 
> Here is an updated version:
> http://cr.openjdk.java.net/~tschatzl/8037344/webrev.1/

Just to make sure that I follow the subtleties in the code here, you set 
_cur_region_cur_card to -1 here. Then when fine_has_next is called it computes 
_cur_region_cur_card + 1 to get 0 and calls _bm.get_next_one_offset?

1115 void HeapRegionRemSetIterator::switch_to_next_prt(PerRegionTable* prt) {
1116   assert(prt != NULL, "Cannot switch to NULL prt");
1117   _fine_cur_prt = prt;
1118 
1119   HeapWord* r_bot = _fine_cur_prt->hr()->bottom();
1120   _cur_region_card_offset = _bosa->index_for(r_bot);
1121 
1122   _cur_region_cur_card = (size_t)-1;
1123 }

The rest of the code change looks ok, but this code is really complex and I 
hope that you've tested this with verification enabled to catch any tricky 
bugs.

/Mikael

> 
> On Fri, 2014-03-14 at 17:41 +0100, Thomas Schatzl wrote:
> > Hi all,
> > 
> >   could I have reviews for this small change that cleans up the
> > 
> > HeapRegionRemSet-iterator?
> > 
> > The reason for this change is that in JDK-7182260 linked together all
> > PerRegionTable data structure (representing parts of the fine remembered
> > set) of a fine remembered set for a particular region. However the
> > iterator still walks the hash table.
> > 
> > I took the opportunity to try to slightly improve the documentation of
> > the HeapRegionRemSetIterator (comments) as the change changes the
> > HeapRegionRemSetIterator interface already anyway.
> > 
> > A micro-benchmark that does nothing for every card indicates that
> > iterating of the PRTs got a lot faster (mainly due to the less
> > complicated check in HeapRegionRemSetIterator::fine_has_next() I assume)
> > and is only a side-effect of the change.
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8037344
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8037344/webrev/
> > 
> > Testing:
> > jprt, specjbb*, specjvm2008*, dacapo and other benchmarks having large
> > remembered sets.
> 
> Thanks,
>   Thomas




More information about the hotspot-gc-dev mailing list