Re: RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics

Liang Mao maoliang.ml at alibaba-inc.com
Wed Feb 19 10:44:19 UTC 2020


Hi Thomas,

When I was testing those benchmarks like specjbb2015 and specjvm2008,
the expansions mostly happened at remark. So I guess the expansion after
concurrent mark at peak usage based on a minimal capacity might
 prevent several expansions in normal young collections. It's only my
 thinking since I don't have much performance data. I don't have any
 problems with expanding after young collection:)

BTW, do you and Stefan prefer to leave the shrink at remark for fixing
the failure of JEP346 and handling the idle scenario?

Thanks,
Liang






------------------------------------------------------------------
From:Thomas Schatzl <thomas.schatzl at oracle.com>
Send Time:2020 Feb. 19 (Wed.) 16:45
To:"MAO, Liang" <maoliang.ml at alibaba-inc.com>; Stefan Johansson <stefan.johansson at oracle.com>; hotspot-gc-dev <hotspot-gc-dev at openjdk.java.net>
Subject:Re: RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics

On 19.02.20 09:09, Liang Mao wrote:
> Hi Thomas and Stefan,
> 
> Regarding the failed test case of JEP 346 and the potential idle
> scenario we discussed, I don't oppose to reserve the shring in
> remark because introducing another perodic GC to make sure the
> mixed GCs may not be a good idea as well.
> 
> Thank Thomas for fixing my mistakes. By looking into your patch,
> I didn't see the expansion after concurrent mark based on
> policy()->desired_bytes_after_concurrent_mark(). Is it missed?
> 

   in an earlier email Stefan asked why the heuristic expands during 
Cleanup. In our opinion this is unnecessary and an artifact of doing 
full gc sizing in the Remark pause.

The reasoning goes as follows: at worst normal expansion between Cleanup 
and the first mixed gc will expand the heap anyway. There does not seem 
to be much difference in doing expansion during Cleanup or GC (or 
inbetween) except that it would arbitrarily move the cost into the 
Cleanup pause. (And in the stable state this shouldn't happen because we 
previously already sized the heap optimally ;) )

So the recent suggestion removes it. As mentioned, this is untested (and 
I am going to look at overnight results later today) but seems okay as 
the last mixed gc will size "optimally" later anyway.

Cleanup pause still records the _minimum_desired_bytes_after_last_gc 
since it is still needed later (and when discussing this last time we 
thought that this is the "best" place, now if we do not expand during 
Cleanup we actually do not need to do that there any more).

One more comment about one of the raised issues with the code further below.

> 
>     ------------------------------------------------------------------
>     From:Thomas Schatzl <thomas.schatzl at oracle.com>
>     Send Time:2020 Feb. 19 (Wed.) 04:52
>     To:"MAO, Liang" <maoliang.ml at alibaba-inc.com>; Stefan Johansson
>     <stefan.johansson at oracle.com>; hotspot-gc-dev
>     <hotspot-gc-dev at openjdk.java.net>
>     Subject:Re: RFR: 8236073: G1: Use SoftMaxHeapSize to guide GC heuristics
> 
[...]
> 
>     - I believe there is an underestimation of the desired bytes after
>     concurrent mark with adaptive IHOP enabled in the current code. If you
>     look at the method G1Policy::desired_bytes_after_concurrent_mark(), the
>     two terms returned by that method do not seem equal. I.e.
>     G1AdaptiveIHOP::predict_unrestrained_buffer_size() does not contain the
>     used bytes, the reserve and other parts used for the static IHOP (i.e.
>     minimum_desired_buffer_size == 0).
> 
>     At most, G1AdaptiveIHOP::predict_unrestrained_buffer_size() covers the
>     young gen part of the latter.

I.e.

size_t G1Policy::minimum_desired_bytes_after_concurrent_mark(size_t 
used_bytes) {
   size_t minimum_desired_buffer_size = 
_ihop_control->predict_unstrained_buffer_size();
   return minimum_desired_buffer_size != 0 ?
            minimum_desired_buffer_size : _young_list_max_length * 
HeapRegion::GrainBytes
          + _reserve_regions * HeapRegion::GrainBytes + used_bytes;

is from what I understand the same as:

if (minimum_desired_buffer_size != 0) {
   return minimum_desired_buffer_size;
} else {
   return _young_list_max_length * ... + reserve_regions...;
}

I *think* the following has been intended:

return (minimum_desired_buffer_size != 0 ?
         minimum_desired_buffer_size : _young_list_max_length * 
HeapRegion::GrainBytes)
         + _reserve_regions * HeapRegion::GrainBytes + used_bytes;

It would be nicer to restructure the code a bit though.

>     As mentioned, currently running more tests until tomorrow (even with
>     above known issues) to get some experience/data to look at with the
>     sizing at mixed gc heuristic.
> 

Thanks,
   Thomas



More information about the hotspot-gc-dev mailing list