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

Thomas Schatzl thomas.schatzl at oracle.com
Tue Feb 11 10:42:34 UTC 2020


Hi,

On 10.02.20 12:47, Liang Mao wrote:
> Hi Thomas,
> 
> In my testing, I didn't change the value of Min/MaxHeapFreeRatio.
> 
> The heap had already shrinked to 5GB but in remark it expand to 6644M.
> The fault value of MinHeapFreeRatio is 40, so the minimal commit size
> after remark is the heap size * 1.67 (3979M * 1.67 = 6644M).
> 1.67 = 100/(100 - 40)
> 
> 
> [1031.322s][info][gc 
> ] GC(741) Pause Young (Concurrent Start) (G1 Evacuation Pause) 4724M->4506M(5120M) 10.607ms
> [1031.322s][info][gc,cpu         ] GC(741) User=0.42s Sys=0.00s Real=0.01s
> [1031.322s][info][gc             ] GC(742) Concurrent Cycle
> [1031.322s][info][gc,marking     ] GC(742) Concurrent Clear Claimed Marks
> [1031.322s][info][gc,marking     ] GC(742) Concurrent Clear Claimed Marks 0.066ms
> [1031.322s][info][gc,marking     ] GC(742) Concurrent Scan Root Regions
> [1031.322s][info][gc,stringdedup ] Concurrent String Deduplication (1031.322s)
> [1031.323s][info][gc,stringdedup ] Concurrent String Deduplication 14224.0B->0.0B(14224.0B) avg 51.1% (1031.322s, 1031.323s) 0.514ms
> [1031.326s][info][gc,marking     ] GC(742) Concurrent Scan Root Regions 3.939ms
> [1031.326s][info][gc,marking     ] GC(742) Concurrent Mark (1031.326s)
> [1031.326s][info][gc,marking     ] GC(742) Concurrent Mark From Roots
> [1031.326s][info][gc,task        ] GC(742) Using 16 workers of 16 for marking
> [1031.483s][info][gc,marking     ] GC(742) Concurrent Mark From Roots 157.144ms
> [1031.483s][info][gc,marking     ] GC(742) Concurrent Preclean
> [1031.484s][info][gc,marking     ] GC(742) Concurrent Preclean 0.404ms
> [1031.484s][info][gc,marking     ] GC(742) Concurrent Mark (1031.326s, 1031.484s) 157.587ms
> [1031.485s][info][gc,start       ] GC(742) Pause Remark
> [1031.496s][info][gc             ] GC(742) Pause Remark 4625M->3979M(6644M) 10.953ms
> [1031.496s][info][gc,cpu         ] GC(742) User=0.22s Sys=0.04s Real=0.01s
> 
> 
> In our production environment, we never use JEP 346 mainly because of 
> JDK version.
> So I cannot tell how if it would work. I agree the "idle" issue is not 
> our main focus for now.
> 
> Using SoftMaxHeapSize to guide adaptive IHOP to make desicion of concurrent
> mark GC cycle can work well with JEP 346 and the resize logic in remark.
> I don't stick to shrink the heap in every GC.
> 
> The capacity in resize_heap_if_necessary will be
> Max2(min_desire_capacity_by_MinHeapFreeRatio, Min2(soft_max_capacity(), 
> max_desire_capacity_by_MaxHeapFreeRatio))
> 
> But both 2 approaches have the problem that default MinHeapFreeRatio is 
> too large
> in remark comparing to full gc.  As resize_heap_if_necessary
> will keep a minimal heap size as 1.667X of used heap size. After remark,
> the used size could be large that not only include those old regions 
> with garbages but
> also the used young regions.
> 
> #############################
> void G1CollectedHeap::resize_heap_if_necessary() {
> ...
> const size_t capacity_after_gc = capacity();
> const size_t used_after_gc = capacity_after_gc - unused_committed_regions_in_bytes();
> #############################
> 
> The used_after_gc is reasonable for full gc but it can contains young 
> regions in remark.
> Do you think it should be changed like this?
> #############################
> const size_t used_after_gc = capacity_after_gc - unused_committed_regions_in_bytes() 
> - young_regions_count() * HeapRegion::GrainWords;
> // young_regions_count is 0 after full GC
> #############################

Apart from naming ("used_after_gc") which has been wrong since that 
method has been in use for Remark, this seems reasonable.

Maybe "old_used_after_gc"? I think the comments need changes to reflect 
that we apply the Min/MaxHeapFreeRatio on the old gen occupancy now 
(which is the same as total occupancy after full gc) because it may be 
called with young regions active.

I also think the whole code that calculates the expansion and shrinking 
amount should be moved to the policy (and g1collectedheap code just 
calling that and then only react on the return value), but that can be 
done separately.

> 
> Besides this, as you suggested, a lower MinHeapFreeRatio would be good.
> But arbitrarily setting a fixed number seems is not a good way that the 
> small number may not meet pause time goal in later young GC. I tried to use
> following number in resize_heap_if_necessary:
> 
> ##############################
> void G1CollectedHeap::resize_heap_if_necessary() {
> ...
> // We can now safely turn them into size_t's.
>    size_t minimum_desired_capacity = (size_t) minimum_desired_capacity_d;
>    size_t maximum_desired_capacity = (size_t) maximum_desired_capacity_d;
> 
> if (!collector_state()->in_full_gc()) {
>      minimum_desired_capacity = MIN2(minimum_desired_capacity, policy()->minimum_desired_bytes(used_after_gc));
>    }

That looks a bit hacky... :) But I do not have a better policy for 
sizing after full gc either. Did you try always using the 
minimum_desired_bytes()?

> 
> ....
> size_t G1Policy::minimum_desired_bytes(size_t used_bytes) const {
>    return _ihop_control->unrestrained_young_size() != 0 ?
>             _ihop_control->unrestrained_young_size() :
>             _young_list_max_length * HeapRegion::GrainBytes
>           + _reserve_regions * HeapRegion::GrainBytes + used_bytes;
> }

I think G1IHOPControl::_target_occupancy (add a getter) is what you want 
to use here (untested).

> #############################
> 
> I made the minimum_desired_capacity small enough based on adaptive IHOP's
> _last_unrestrained_young_size. Even without SoftMaxHeapSize, the test can
> keep the memory under 3GB. It's a rough example and I didn't predict the 
> promotion bytes of next young gc yet. Do you think
> a proper value of minimum_desired_capacity in remark resize
> +
> G1AdaptiveIHOPControl::actual_target_threshold according to 
> soft_max_capacity> is enough?

Yes, both fixing the resizing logic and changing the IHOP target (and 
young gen size) according to SoftMaxHeapSize should be sufficient to let 
G1 keep that goal without too many commit activity.

The resizing logic change could be handled under JDK-8238686, although 
this change does not modify the use of MaxHeapFreeRatio.

There is a cleaned up version of my earlier change that implements the 
latter at http://cr.openjdk.java.net/~tschatzl/8236073/webrev.1/ .

I will test your suggested changes and see its impact on our perf suite.

Thanks a lot,
   Thomas

P.S: it would be nice to send diffs of suggested changes for easier 
application too.


More information about the hotspot-gc-dev mailing list