RFR (S/M): 8178105: Switch mark bitmaps during Remark

[Thomas Schatzl]

Hi,

  thanks for your patience...

On Tue, 2018-04-03 at 14:49 +0200, Stefan Johansson wrote:
> 
> On 2018-04-03 14:25, Thomas Schatzl wrote:
> > Hi Stefan,
> > 
> > On Tue, 2018-04-03 at 12:58 +0200, Stefan Johansson wrote:
> > > Hi Thomas,
> > > 
> > > On 2018-03-29 16:35, Thomas Schatzl wrote:
> > > > Hi all,
> > > > [...]
> > > > 
> > > > CR:
> > > > https://bugs.openjdk.java.net/browse/JDK-8178105
> > > > Webrev:
> > > > http://cr.openjdk.java.net/~tschatzl/8178105/
> > > 
> > >   Looks good, but a few comments:
> > > 
[...]
> > I added a different kind of check that verifies what you probably
> > thought of, and that is that the number of words to add to the
> > current region must always be larger than zero. If it is zero,
> > there is something really wrong. 
> 
> Exactly, that's what I wanted. But I don't see that check now, just 
> having a assert(marked_words != 0, "") on row 1032 would be enough.

My fault. I did not upload the latest version of the change :( It is up
now, the only difference to previous is line 

1036       assert(words_to_add > 0, "Out of space to distribute before
end of humongous object in region %u (starts %u)", i, region_idx);

But there is a new webrev anyway... in the 1_to_2 webrev it is line
1033

> > > 
> > > 1047       log_trace(gc)("Added " SIZE_FORMAT " words to region
> > > %u
> > > (%s)", marked_words, region_idx, hr->get_type_str());
> > > I think the logging should use at least one more tag, maybe
> > > "region" or "marking", but you probably know what makes most
> > > sense in this area. I also think it would be nice if we got this
> > > same log-line for all regions. Now we get this for "normal"
> > > regions, but we get "Distributing..." for humongous start regions
> > > and "Not Added..." for humongous continuous. So I would suggest
> > > adding the same log-line  to the distribute-function after line
> > > 1034.
> > 
> > Done.
> 
> Sorry for being picky but the current solution won't print anything
> for continuous humongous. Adding the log to distribute_marked_bytes()
> would solve this and we would still get info on region type so I see
> no need for:

Okay. I think I got your intention right this time :)

> 1052         log_trace(gc, marking)("Adding " SIZE_FORMAT " words to 
> humongous start region %u (%s), word size %d (%f)",
> 1053                                marked_words, region_idx, 
> hr->get_type_str(),
> 1054                                oop(hr->bottom())->size(), 
> (double)oop(hr->bottom())->size() / HeapRegion::GrainWords);
[...]
> > All fixed.
> > 
> > Webrevs:
> > http://cr.openjdk.java.net/~tschatzl/8178105/webrev.0_to_1/ (diff)
> > http://cr.openjdk.java.net/~tschatzl/8178105/webrev.1/ (full)
> 
> Apart from the small things above this looks great.

http://cr.openjdk.java.net/~tschatzl/8178105/webrev.1_to_2/ (diff)
http://cr.openjdk.java.net/~tschatzl/8178105/webrev.2/ (full)

Ran gc/g1 tests successfully with that change.

Thanks,
  Thomas