Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-7089625

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: hs22
    • Fix Version/s: hs22
    • Component/s: hotspot
    • Subcomponent:
      gc
    • Resolved In Build:
      b06
    • CPU:
      generic
    • OS:
      generic
    • Verification:
      Verified

      Backports

        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.

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  tonyp Tony Printezis
                  Reporter:
                  tonyp Tony Printezis
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  1 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Imported:
                    Indexed: