Request for Review (s) - 7012980: PSOldGen is increased if there is no space in Metaspace

[Jon Masamitsu]

Kim,

On 5/8/2015 1:26 PM, Kim Barrett wrote:
> On May 8, 2015, at 11:58 AM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>> When full collections were being done because the Metaspace threshold was
>> reached, the Java heap was being expanded.  Changed the test for expanding
>> the Java heap to check that the GCCause was for a full old gen or for adaptive
>> size policy which is done in anticipation that a full GC will be needed before
>> the next young gen collection.
>>
>> This change is done in two parts: the fix for this bug and a cleanup
>> to add the GCCause::is_system_gc().  This can be pushed separately
>> if desired.
>>
>> http://cr.openjdk.java.net/~jmasa/7012980/webrev.fix.00/
>> http://cr.openjdk.java.net/~jmasa/7012980/webrev.is_system_gc.00/
>>
>> https://bugs.openjdk.java.net/browse/JDK-7012980
>>
>> Stability tested product and debug with gc_test_suite.  Performance tested
>> with specjbb2000 and specjbb2005 with and without MetaspaceSize set to
>> generate some Metaspace threshold caused full GC's.
>>
>> Thanks.
>>
>> Jon
> webrev.is_system_gc.00 looks good.
>
> webrev.fix.00:
>
> ------------------------------------------------------------------------------
> src/share/vm/gc_interface/gcCause.hpp
>    93     assert(cause != GCCause::_old_generation_too_full_to_scavenge &&
>    94            cause != GCCause::_old_generation_expanded_on_last_scavenge,
>    95     err_msg("This GCCause may be correct but is not expected yet: %s",
>    96       to_string(cause)));

Changed to

  93     assert(cause != GCCause::_old_generation_too_full_to_scavenge &&

  94       cause != GCCause::_old_generation_expanded_on_last_scavenge,

  95     err_msg("This GCCause may be correct but is not expected yet: %s",

  96       to_string(cause)));


>
> The err_msg expression is oddly indented.
>
> Does this mean there is a plan to actually use those two causes?  If
> not, then how about removing them, rather than checking for them.
> There presently appear to be no real uses of them.
>
> ------------------------------------------------------------------------------

GCCause::_old_generation_too_full_to_scavenge should be used in
GenCollectorPolicy::satisfy_failed_allocation() at

  754   } else {
  755     if (Verbose && PrintGCDetails) {
  756       gclog_or_tty->print(" :: Trying full because partial may 
fail :: ");
  757     }

GCCause::_old_generation_expanded_on_last_scavenge

would be used in

  PSScavenge::invoke()

237   if (need_full_gc) {
238     GCCauseSetter gccs(heap, GCCause::_adaptive_size_policy);
239     CollectorPolicy* cp = heap->collector_policy();
240     const bool clear_all_softrefs = cp->should_clear_all_soft_refs();

_adaptive_size_policy is currently used but is less precise than
_old_generation_expanded_on_last_scavenge

There's no CR on this.  If you think it is worth keeping, I'll open one.

Thanks.

Jon