RFR (S): 8037344: Use the "next" field to iterate over fine remembered instead of using the hash table
Thomas Schatzl
thomas.schatzl at oracle.com
Mon Apr 14 09:08:23 UTC 2014
Hi Bengt,
On Mon, 2014-04-14 at 10:13 +0200, Bengt Rutisson wrote:
> Hi Thomas,
>
> Looks good. Nice work!
>
> A couple of minor nits:
>
> Both _cur_region_cur_cards and HeapRegion::CardsPerRegion are size_t, so
> there are a few unnecessary casts in the code I think:
>
> 1058 _cur_region_cur_card((size_t)HeapRegion::CardsPerRegion),
>
> 1095 if (_cur_region_cur_card == (size_t)HeapRegion::CardsPerRegion) {
>
> I think the message for this assert is a bit strange.
>
> 1107 guarantee(_cur_region_cur_card < HeapRegion::CardsPerRegion,
> "card index must be within heap");
>
> We are not verifying that the card index is within the heap. It is
> verifying that the current card is within a heap region.
All fixed.
> Actually this raises another question. The name _cur_region_cur_card
> lead me to believe that the region was somehow encoded in this value,
> but it seems to only be the current card index (for the region that we
> have stored in _finc_cur_prt). How about changing the name to just
> _cur_card? Not sure it is better. I just got confused by the name, but I
> do realize that the intent is that it is the current card index for the
> regions we are iterating over.
I renamed it to _cur_card_in_prt. Note that a PRT corresponds to a
single region at this time, so the original does not sound too far
fetched.
> The method switch_to_next_prt() actually don't step to the next or
> verify that it is switching to the next PRT. Can we rename it to just
> switch_to_prt() ?
Done.
Webrev:
http://cr.openjdk.java.net/~tschatzl/8037344/webrev.3/
Incremental webrev:
http://cr.openjdk.java.net/~tschatzl/8037344/webrev.3.diff/
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list