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).