CRR (XS): 7089625: G1: policy for how many old regions to add to the CSet (when young gen is fixed) is broken

Tony Printezis tony.printezis at
Mon Sep 12 16:09:30 UTC 2011

I had to re-arrange some code when I added the ergo decision output to 
G1 and I unfortunately got a condition wrong which broke the policy of 
how many old regions to add to the CSet.

Could I please get a couple of quick code review for the two-line fix?

I attached a description of the issue (from the CR) below.



The original code was:

      should_continue =
        ( hr != NULL) &&
        ( (adaptive_young_list_length()) ? time_remaining_ms > 0.0
          : _collection_set_size < _young_list_fixed_length );

In the fix for 7050392 I refactored it to be able to emit the correct 
ergo policy output:

      should_continue = true;
      if (hr == NULL) {
        // No need for an ergo verbose message here,
        // getNextMarkRegion() does this when it returns NULL.
        should_continue = false;
      } else {
        if (adaptive_young_list_length()) {
          if (time_remaining_ms < 0.0) {
            ergo_verbose1(ErgoCSetConstruction, ...);
            should_continue = false;
        } else {
          if (_collection_set_size < _young_list_fixed_length) {
            ergo_verbose2(ErgoCSetConstruction, ...);
            should_continue = false;

and unfortunately I didn't negate the _collection_set_size < 
_young_list_fixed_length condition. The intention of this code is: if hr 
!= NULL (which means: we've just found and added an old region to the 
CSet) and !adaptive_young_list_length() (which means: the young gen size 
is fixed), we should carry on adding old regions to the CSet until the 
CSet length (_collection_set_size) reaches the fixed young list target 
length (_young_list_fixed_length). So, should_continue should be set to 
false when _collection_set size >= _young_list_fixed_length, not when 
_collection_set_size < _young_list_fixed_length.

More information about the hotspot-gc-dev mailing list