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

Remove GCLockerInvokesConcurrent

    Details

    • Type: CSR
    • Status: Closed
    • Priority: P4
    • Resolution: Approved
    • Fix Version/s: 14
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
      gc
    • Compatibility Kind:
      behavioral
    • Compatibility Risk:
      minimal
    • Compatibility Risk Description:
      Hide
      Specifying a non-default value for the option has either required combination with a deprecated and now removed collector (CMS), or has never worked properly (G1). A web search for the option found no interesting mentions. There are no JDK tests of the option.
      Show
      Specifying a non-default value for the option has either required combination with a deprecated and now removed collector (CMS), or has never worked properly (G1). A web search for the option found no interesting mentions. There are no JDK tests of the option.
    • Interface Kind:
      add/remove/modify command line option
    • Scope:
      Implementation

      Description

      Summary

      Obsolete and later remove the -XX:GCLockerInvokesConcurrent product VM option.

      Problem

      -XX:GCLockerInvokesConcurrent is a boolean product VM option, defaulting to false. It has only ever affected the behavior of the CMS and G1 collectors. Normally, when a GC-locker induced collection is required only a partial collection is performed. (This is true for all of the generational collectors in HotSpot.) For CMS and G1, when this option is true (not the default), a GC-locker induced collection instead attempts to start a concurrent full collection.

      It's not clear why the option of doing a concurrent full collection instead of a partial collection was ever provided. Even the change that introduced this option for CMS (JDK-6919638) describes it as "may be a suboptimal policy" and "left off by default until a more nuanced dynamic policy is designed in the future".

      Normally the 3-step policy of deprecate/obsolete/remove would be taken, since this is a product option. However, we propose in this case to skip the deprecation step and go straight to obsolete.

      The rationale for skipping the deprecation step is

      (1) Usage with CMS has been deprecated for some time, since CMS was deprecated. CMS was recently removed (JDK-8232365), so the option now only affects G1.

      (2) A bug in the G1 implementation of this feature (JDK-8233279), until recently (2019-11-13) made it unusable with G1.

      Solution

      Remove the sole use of -XX:GCLockerInvokesConcurrent by G1. Remove a couple of comments in G1 code that refer to the option. Remove the option definition and add the name to the obsolete options list.

      Specification

      https://cr.openjdk.java.net/~kbarrett/8233280/open.00/

      --- old/src/hotspot/share/gc/g1/g1CollectedHeap.cpp 2019-11-13 23:26:50.003663136 -0500
      +++ new/src/hotspot/share/gc/g1/g1CollectedHeap.cpp 2019-11-13 23:26:49.751649570 -0500
      @@ -2001,7 +2001,6 @@
      
       bool G1CollectedHeap::should_do_concurrent_full_gc(GCCause::Cause cause) {
         switch (cause) {
      -    case GCCause::_gc_locker:               return GCLockerInvokesConcurrent;
           case GCCause::_g1_humongous_allocation: return true;
           case GCCause::_g1_periodic_collection:  return G1PeriodicGCInvokesConcurrent;
           default:                                return is_user_requested_concurrent_full_gc(cause);
      @@ -2281,8 +2280,7 @@
         } else if (GCLocker::should_discard(cause, gc_count_before)) {
           // Indicate failure to be consistent with VMOp failure due to
           // another collection slipping in after our gc_count but before
      -    // our request is processed.  _gc_locker collections upgraded by
      -    // GCLockerInvokesConcurrent are handled above and never discarded.
      +    // our request is processed.
           return false;
         } else if (cause == GCCause::_gc_locker || cause == GCCause::_wb_young_gc
                    DEBUG_ONLY(|| cause == GCCause::_scavenge_alot)) {
      --- old/src/hotspot/share/gc/g1/g1CollectedHeap.hpp 2019-11-13 23:26:51.063720200 -0500
      +++ new/src/hotspot/share/gc/g1/g1CollectedHeap.hpp 2019-11-13 23:26:50.807706419 -0500
      @@ -262,12 +262,11 @@
      
         // Return true if an explicit GC should start a concurrent cycle instead
         // of doing a STW full GC. A concurrent cycle should be started if:
      -  // (a) cause == _gc_locker and +GCLockerInvokesConcurrent,
      -  // (b) cause == _g1_humongous_allocation,
      -  // (c) cause == _java_lang_system_gc and +ExplicitGCInvokesConcurrent,
      -  // (d) cause == _dcmd_gc_run and +ExplicitGCInvokesConcurrent,
      -  // (e) cause == _wb_conc_mark,
      -  // (f) cause == _g1_periodic_collection and +G1PeriodicGCInvokesConcurrent.
      +  // (a) cause == _g1_humongous_allocation,
      +  // (b) cause == _java_lang_system_gc and +ExplicitGCInvokesConcurrent,
      +  // (c) cause == _dcmd_gc_run and +ExplicitGCInvokesConcurrent,
      +  // (d) cause == _wb_conc_mark,
      +  // (e) cause == _g1_periodic_collection and +G1PeriodicGCInvokesConcurrent.
         bool should_do_concurrent_full_gc(GCCause::Cause cause);
      
         // Attempt to start a concurrent cycle with the indicated cause.
      --- old/src/hotspot/share/gc/shared/gc_globals.hpp  2019-11-13 23:26:52.043772958 -0500
      +++ new/src/hotspot/share/gc/shared/gc_globals.hpp  2019-11-13 23:26:51.787759176 -0500
      @@ -228,10 +228,6 @@
                 "A System.gc() request invokes a concurrent collection; "         \
                 "(effective only when using concurrent collectors)")              \
                                                                                   \
      -  product(bool, GCLockerInvokesConcurrent, false,                           \
      -          "The exit of a JNI critical section necessitating a scavenge, "   \
      -          "also kicks off a background concurrent collection")              \
      -                                                                            \
         product(uintx, GCLockerEdenExpansionPercent, 5,                           \
                 "How much the GC can expand the eden by while the GC locker "     \
                 "is active (as a percentage)")                                    \
      --- old/src/hotspot/share/runtime/arguments.cpp 2019-11-13 23:26:53.007824853 -0500
      +++ new/src/hotspot/share/runtime/arguments.cpp 2019-11-13 23:26:52.747810857 -0500
      @@ -618,6 +618,7 @@
         { "ResizeOldPLAB",                           JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(15) },
         { "UseCMSBestFit",                           JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(15) },
         { "UseCMSInitiatingOccupancyOnly",           JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(15) },
      +  { "GCLockerInvokesConcurrent",     JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(15) },
         { "BindGCTaskThreadsToCPUs",       JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(16) },
         { "UseGCTaskAffinity",             JDK_Version::undefined(), JDK_Version::jdk(14), JDK_Version::jdk(16) },

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                kbarrett Kim Barrett
                Reporter:
                kbarrett Kim Barrett
                Reviewed By:
                Stefan Johansson, Thomas Schatzl
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: