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

[Liang Mao]

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?

Thanks,
Liang






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

Hi Liang,

   dug through the changes a bit, took longer and only managed to do 
cursory testing as there were a few issues.

That (very) cursory testing showed that memory consumption on one 
specjvm2008 out-of-box application is as baselined, but currently 
running the full set.

The change I used is available at 
http://cr.openjdk.java.net/~tschatzl/8236073/webrev.2/, I will step 
through what changed below.

- not really a bug and pre-existing, but I changed the various 
resize_heap_* to always include the exact GC pause because particularly 
for the "after_concurrent_mark" suffix it is not clear what this means. 
I.e. in the Remark or Cleanup pauses, or at the real end of concurrent 
cycle (still concurrent)?

This has not been done consistently yet.

- I think there has been a copy&paste error in 
G1CollectedHeap::resize_heap_if_necessary, the two calculations to 
determine the min and max desired capacity were equal. I.e.

1178   size_t minimum_desired_capacity = 
_heap_sizing_policy->target_heap_capacity(used_after_gc, MinHeapFreeRatio);
1179   size_t maximum_desired_capacity = 
_heap_sizing_policy->target_heap_capacity(used_after_gc, MinHeapFreeRatio);

Note the duplicate use of MinHeapFreeRatio. Fixed in above webrev.

- CollectorState contains flags that basically indicate they type of GC, 
which should be set at the start of gc and updated at the end of gc. The 
new finish_of_mixed_gc does not fit here as it is basically a flag 
indicating that we need to do the resizing.

The previous implementation also lets the first young-only gc after the 
last mixed gc do the resizing which is probably not as intended.

By adding an additional policy()->next_gc_should_be_mixed() call instead 
of the state check (and removing this pause state/type completely) fixes 
this (I think ;)).

- the suggested change removes the expansion during Cleanup for the 
reasons stated earlier. This removes the need for some code in the 
G1HeapSizingPolicy where originally _minimum_desired_bytes_after_last_cm 
had been stored. It's better to move this to G1Policy (and pre-existing, 
G1Policy should be the owner of G1HeapSizingPolicy which I did not fix 
in this change)

- (the suggested change does not add the shrinking at remark discussed 
earlier; I still think it would be nice and maybe fix that failing 
regression test)

- there should be more gc+heap+ergo logging of calculated 
targets/desired sizes in the new methods in G1HeapSizingPolicy, 
otherwise the decisions are very hard to follow after the fact.

- 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. Some better name for this should be found 
too =)

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