RFR (S): 8069760: When iterating over a card, G1 often iterates over much more references than are contained in the card

[Kim Barrett]

On Jan 27, 2015, at 8:21 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8069760
> 
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8069760/webrev/
> 
> Testing:
> nosql, dev-submit performance testing (specj*), vm.quick.testlist, jprt.

------------------------------------------------------------------------------ 
src/share/vm/gc_implementation/g1/heapRegion.cpp 
 443     assert(obj == oop(cur), "Loop invariant");

This and later references to "obj" in the first iteration depend on
the assignment of "obj" in the loop back at
 425     obj = oop(cur);

It required some careful study to conclude that "obj" is guaranteed to
be initialized and have the proper value at line 443.  I think it
would be better to move the update of "obj" from the end of the loop
(line 463) to the head of the loop (after line 440).  Of course, then
eliminate the unnecessary assert(obj == oop(cur), ...).

------------------------------------------------------------------------------ 
src/share/vm/gc_implementation/g1/heapRegion.cpp 
 453       // Non-object arrays are sometimes marked imprecise at the object start. We

I think "Non-object arrays" would be clearer as "Non-objArrays".  At
least, I think that's the intent.  As written I read it as arrays of
non-objects, e.g. primitive arrays such as int[].

------------------------------------------------------------------------------
src/share/vm/gc_implementation/g1/heapRegion.cpp 
 440   do {
 441     assert(obj->klass_or_null() != NULL, "Loop invariant");
...
 444     if (obj->klass_or_null() == NULL) {

Line 444 can never be true if the assertion at line 441 is correct.

The assertion block from line 441-443 used to be before the loop, and
has now been hoisted into the loop.  I'm not sure that's correct, and
this inconsistency is a large part of what makes that move look
suspect.  Or maybe it should be moved after the
  if (obj->klass_or_null() == NULL)
check?  But then we might be missing the first iteration assertion
that the klass is != NULL.

------------------------------------------------------------------------------
src/share/vm/gc_implementation/g1/heapRegion.cpp 
 678 //      gclog_or_tty->print_cr("Verifying address " PTR_FORMAT, p2i(p));

Leftover debug print statement.