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

G1 concurrent gc upgrade to full gc not working

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P3
    • Resolution: Fixed
    • Affects Version/s: 14, 15, 16, 17
    • Fix Version/s: 17
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
      gc
    • Introduced In Version:
      14
    • Resolved In Build:
      b26

      Description

      During the review of JDK-8268163 we noticed that the upgrade to full gc in VM_G1TryInitiateConcMark::doit() is never true, and so the upgrade will never happen:

      The code is something like this:

        } else if (g1h->should_upgrade_to_full_gc(_gc_cause)) {
          _gc_succeeded = g1h->upgrade_to_full_collection();
        } else {
          _gc_succeeded = true;
        }

      with G1CollectedHeap::upgrade_to_full_gc() doing this:

        if (should_do_concurrent_full_gc(_gc_cause)) {
          return false;
        } else if (has_regions_left_for_allocation()) {

      but VM_G1TryInitiateConcMark is only ever executed with should_do_concurrent_full_gc() == true, so the result of upgrade_to_full_gc is always false.

      This bug has been introduced with some refactoring of G1 VM operations in JDK-8232588.

      However, there is something odd about the whole feature added in JDK-8195158 which is written as if the intention had been to *not* do this (https://bugs.openjdk.java.net/browse/JDK-8195158?focusedCommentId=14147598&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14147598) states:

      " The fix (which is being worked on) is to not upgrade user initiated GCs that are supposed to be concurrent. "

      and the code *adds* this functionality too.

      So either this is a bug because JDK-8232588 disabled this functionality, or this is a bug because since the upgrade is impossible, there is superfluous code.

      I think we should upgrade for user-initiated system.gc() on the expectation that at least some memory should be freed; in that case removing the first check in should_upgrade_to_full_gc() should be good (in both usages).

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              sjohanss Stefan Johansson
              Reporter:
              tschatzl Thomas Schatzl
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: