JDK-7089625 : G1: policy for how many old regions to add to the CSet (when young gen is fixed) is broken
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: hs22
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-09-12
  • Updated: 2013-10-04
  • Resolved: 2011-11-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 Other
7u2Fixed 8Fixed hs22Fixed
Related Reports
Relates :  
Description
Unfortunately, with the fix for:

7050392: G1: Introduce flag to generate a log of the G1 ergonomic decisions 

I accidentally broke the policy that decides how many old regions to add to the CSet when the young gen is fixed. 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.

As is, the code can do the wrong thing in a couple of ways:

- Only add a single old region to the CSet and exit (even though we can add many more).
- If the young length is too small and there's space for only one old region, we'll reach the target after adding a single old region to the CSet and we'll then end up adding the remaining old regions irrespective of how long the CSet will be.

Comments
EVALUATION See main CR
24-09-2011

EVALUATION http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/f1b4e0e0bdad
13-09-2011

EVALUATION See Description and Suggested Fix.
12-09-2011

SUGGESTED FIX - if (_collection_set_size < _young_list_fixed_length) { + if (_collection_set_size >= _young_list_fixed_length) { ergo_verbose2(ErgoCSetConstruction, "stop adding old regions to CSet", - ergo_format_reason("CSet length lower than target") + ergo_format_reason("CSet length reached target") ergo_format_region("CSet") ergo_format_region("young target"), _collection_set_size, _young_list_fixed_length); should_continue = false;
12-09-2011