Request for review (S): 7029036: Card-table verification hangs with all framework collectors, except G1, even before the first GC

[Jesper Wilhelmsson]

Is there any particular reason to why you changed it from a while to a for?
Either way it looks good. (But it would have looked even better in reviewboard ;))
Ship it!
/Jesper

On 2011-03-24 09:15, Y. Srinivas Ramakrishna wrote:
>
> I'd like a few reviews for a fix to the following CR:-
>
> 7029036: Card-table verification hangs with all framework collectors, except
> G1, even before the first GC
>
> Problem: This is a day one performance bug in card-table verification
> infrastructure
> in the so-called "gc framework", and affects CMS, ParNew and SerialGC.
> Details on why the problem was unmasked only recently can be found in
> the bug report. In a nutshell, existing code has quadratic complexity
> when scanning large arrays because for an object of size O(n)
> and with k clean card ranges, the cost is O(k.n), which with O(n)
> worst case clean card ranges would yield O(n^2) cost.
>
> Solution: By using a memory-interval-bounded iteration over the
> clean card ranges of these objects, the complexity is returned
> to O(k) which is O(n), because we do not end up scanning an
> entire prefix of the object each time before scanning each
> clean range.
>
> I also simplified the verification closure (for the product build)
> by ensuring that the iterator never returns reference locations
> outside the range of clean cards to the closure, which itself just
> asserts in non-product builds that this is always the case.
> (This assertion allowed us to find a few memory-interval-delimited
> iteration methods which were not respecting the memory-interval provided.
> That is being separately fixed in CR 7034035; stay tuned.)
>
> Testing: The tests listed in the bug report with +VerifyBefore/AfterGC
> to verify that the heap verification now does not "hang" but rather
> completes very quickly even with much larger heaps. Also refworkload
> on solaris/x86 and JPRT testing (ongoing) with
> heap verification switched on.
>
> webrev: http://cr.openjdk.java.net/~ysr/7029036/webrev.00/
>
> Thanks for your reviews!
> -- ramki