RFR(M): 7114678: G1: various small fixes, code cleanup, and refactoring

Bengt Rutisson bengt.rutisson at oracle.com
Mon Jul 16 13:31:55 UTC 2012

Hi John,

This looks good to me. Thanks for preparing the change.

A couple of comments:


The new class CSetChooserParUpdater is only instantiated in one place 
(in the ParKnownGarbageHRClosure constructor). There it always get true 
for the parallel value. Maybe we can remove the !parallel case from 


A matter of taste. But I think it is difficult to follow the code when 
we have overloaded version of all of these methods: is_obj_dead(), 
is_obj_ill() and is_obj_dead_cond(). Would it be cleaner to have 
is_obj_dead_cond(const oop obj, const HeapRegion* hr, const VerifyOption 
vo) accept NULL as hr and let the methods is_obj_dead() and is_obj_ill() 
handle that they get hr==NULL ? That would mean that we would not need 
any of these methods to be overloaded to take hr or not.


How about replacing the HR_FORMAT and HR_FORMAT_PARAMS macros with a 
HeapRegion::to_string() method?

Otherwise it looks good to me. Nice cleanups!


On 2012-07-06 19:48, John Cuthbertson wrote:
> Hi Everyone,
> This is the first in a series of patches that were completed by Tony 
> just before he left Oracle. There will be a few more in the coming few 
> weeks.
> This first patch is a pre-cursor to another patch that introduces 
> iterators for heap regions, and is exactly what it says in the 
> synopsis (and description in the bug database). Tony sent a version of 
> this patch earlier in the year; I reviewed it and Tony incorporated 
> the feedback into this version. So I'm looking for a couple of 
> volunteers to also review these changes. The webrev can be found at: 
> http://cr.openjdk.java.net/~johnc/7114678/webrev.0/
> Testing: GC Test Suite with and without some stress options, NSK tests 
> (regression, gc, jit, and runtime), jprt.
> Thanks,
> JohnC

More information about the hotspot-gc-dev mailing list