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

XMLWordPrintable

    • gc
    • b06
    • generic
    • generic
    • Verified

        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.

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

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: