RFR (XL): 8151386: Extract card live data out of G1ConcurrentMark

[Thomas Schatzl]

Hi all,

  can I have reviews for this (large) change that moves the "Liveness
Count Data" from the g1ConcurrentMark files into their own class, and
following that, cleaning it up?

In JDK-8077144 the way we generate the per-card live data used for
various liveness analysis (well, remembered set scrubbing for now)
significantly.
At the same time it showed that it would be a very good idea to move it
out into a separate class, to be, in the future be able to improve the
information it contains (JDK-8151846) and its representation more
easily.
This is the change that tries to do that exactly, followed by some
cleanup.

Improvements to the code:

- add timing log output for related work where that has been missing

- the new class name that contains this information has been changed to
G1CardLiveData to indicate that it contains liveness information on a
card basis.

- the owner of G1CardLiveData is G1RemSet: at the moment only the
remembered set uses it for scrubbing data. That may change in the
future as necessary.

- instead of passing around multiple bitmaps for scrubbing, only pass
around that G1CardLiveData, which with its given API makes it nicer to
use (imo).

- fix the memory leak Kim critized in the very first review for
8077144.

- tried to improve general naming etc.

- incorporated lots of Erik H.'s cleanup changes he made for his review
for 8077144 we talked about in private.

A (consciously) remaining wart:

- the region live bitmap (G1CardLiveData::_live_regions_bm) uses
allocate_large_bitmap() to be allocated in virtual memory. This is kind
of a waste of memory, as the region live bitmap is typically very small
(a few thousand bits).
However, the region live bitmap will be removed in the future, as it
will be first replaced by a live card count in JDK-8151846 (the next
RFR I have in the pipeline), and the coarse bitmap as its only user
removed during rememberd set refactoring (JDK-8017163) reviews which
will probably take one or two more RFRs. So I did not want to spend
effort optimizing that.

Similarly, in the review for 8077144 Mikael commented about the
handling of the _live_regions_bm in G1CardLiveDataHelper:

"   void set_bit_for_region(HeapRegion* hr) {
     BitMap::idx_t index = (BitMap::idx_t) hr->hrm_index();
     _region_bm->par_at_put(index, true);
   }"

I'm not convinced that the _region_bm even needs to be part of the
"helper".
Perhaps it would be cleaner if the helper class was only responsible
for the card bitmap and the modification of the region bit map was
moved elsewhere?"

Again, the region live bitmap will go away soon, additionally moving
that to somewhere else would likely mean the need to allow access to
that method by more classes. I would like to avoid that, and make
G1CardLiveData read-only from outside (in the public interface).

CR:
https://bugs.openjdk.java.net/browse/JDK-8151386

To ease review a little, I split this change into two parts, that I
intend to push as a single change: first, trying to move the
information out of G1ConcurrentMark, and second, try to brush up the
code a little.

http://cr.openjdk.java.net/~tschatzl/8151386/webrev.0a/ contains the
"move" with adaptations to make it work. (Note that I know that this
change alone leaks memory with -XX:+VerifyDuringGC as the temporary
bitmaps used there are not freed)

http://cr.openjdk.java.net/~tschatzl/8151386/webrev.0b/ is the change
that tries to improve the code quality and fixes the mentioned issue.

http://cr.openjdk.java.net/~tschatzl/8151386/webrev.0/ contains the
full change.

This change is based on the changes for JDK-8077144.

Testing:
jprt multiple times (partially with other changes), vm.gc with 
-XX:+VerifyDuringGC specifically and with other changes. Some micro-
and macro-benchmarks.

Thanks a lot,
  Thomas