RFR: 8059066: CardTableModRefBS might commit the same page twice

[Erik Helin]

On 2014-11-20, Kim Barrett wrote:
> On Nov 20, 2014, at 1:27 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> > 
> > On Nov 17, 2014, at 12:43 PM, Erik Helin <erik.helin at oracle.com> wrote:
> >> 
> >> Because the start is aligned down and the end is aligned up, the two
> >> committed regions will always share at least one page (since the start
> >> for _committed[1] will be aligned down and the end for _committed[0]
> >> will be aligned up).
> > 
> > I’m just getting back to this today, and haven’t yet refreshed my understanding
> > of the code.  However, the claim that there will be at least one shared page,
> > (with followup reasoning for exactly one shared page), doesn’t seem right to me.
> > It seems to me there could be zero shared pages, since _committed[1] could
> > (perhaps accidentally, if not by design) be page aligned.
> 
> Hm, and it seems that committed regions are always page aligned.  I’m clearly not
> understanding something yet.  Back to reading code…

Me and Kim discussed this offline and we both now agree that the existing
patch does work, but the check if the first _committed region expands into
the second _committed region can be simpler. The existing check in the
patch is:

  if (new_end_aligned > _committed[ri].start() &&
      new_end_aligned <= _committed[ri].end()) {

The check if new_end_aligned <= _committed[ri].end() is not necessary
because this must always be the case. This is because _committed[0] can't
expand further than the original, unaligned start of _committed[1] and
_committed[1] will always commit at least one page in the byte array for
the card table (starting at _committed[ri].start()). Therefore, it is
enough to check:

  if (new_end_aligned > _committed[ri].start()) {

I updated the patch to use this simple check and also added an assert to
show that new_end_aligned <= _committed[ri].end() always is true. Please
see new webrevs at:
- full: http://cr.openjdk.java.net/~ehelin/8059066/webrev.01/index.html
- inc:  http://cr.openjdk.java.net/~ehelin/8059066/webrev.00-01/index.html

Thanks,
Erik