8262185: Some questions regarding prune collection set candidates early

[Thomas Schatzl]

Hi,

On 09.05.22 16:25, Tianqi Xia wrote:
> Hi,
> 
> I tried to backport and test this patch with JDK11 on my local machine.
> After going through the code(in master branch), I had 2 minor questions
> related to the implementation details.
> 
> 1. Seems to me one of the pruning conditions is "we must keep at least
> min_old_cset_length number of regions in the cset". The formula for
> calculating min_old_cset_length is dividing the number of candidate regions
> by G1MixedGCCountTarget. This calculation happens twice during GC, once in
> this pruning logic, followed by the other one in finalizing the cset old
> regions(G1Policy::calculate_old_collection_set_regions). After pruning, the
> number of candidate regions will be decreased, which means the value of
> min_old_cset_length calculated during calculate_old_collection_set_regions
> will always be less than the one calculated during pruning, is that
> intended?

The pruning during the Cleanup pause is intended as a rough conservatige 
approximate reduction of the number of candidates for which g1 is going 
to maintain the remembered set for the whole space reclamation/mixed gc 
phase.

The use of the minimum old cset length is some fairly random and crude 
heuristic to, in case there are very few regions that are candidates, to 
at least do some work in the mixed phase.

The alternative would be to not do anything, but that could lead to 
out-of-memory before the next (old gen) space reclamation/mixed gc phase.

I do not think the use of minimum old gen cset has a particular meaning 
except to bound the minimum number of regions to take in that case; it's 
better than some even more random fixed number or another random option 
that nobody uses (and understands).

Ideally there would be some mechanism to determine exactly how much 
space (and calculate the number of regions from that given the 
candidates) we would need to reclaim to be able to get to the next space 
reclamation phase without going into evacuation failures/full gc. This 
value could be used as a better minimum region value - for better 
throughput you would probably want to take (much) more anyway, 
particularly if these regions after the minimum are still easy to 
reclaim, but still it would be a better minimum.

The current heuristic is dodgy, but apparently it works well enough. 
More investigation in edge cases like this (or investigations into the 
above suggested alternative, or your suggestions :)) would be 
appreciated of course.

> 
> 2. The second question is about the following if statement in the
> do_heap_region function of G1PruneRegionClosure:
>      if (_num_pruned > _max_pruned ||
>          _cur_wasted + reclaimable > _max_wasted) {
>          return true;
>      }
>     Should it be _num_pruned >= _max_pruned? Seems like we should stop if we
> have already pruned _max_pruned number of regions.
> 

That's right, makes more sense and should be fixed. I filed JDK-8286467. 
If you do not pick it up, I'll this week.

The impact will be limited though as the point of the pruning is 
excessive management overhead for remembered sets; one region more or 
less does not really matter here, neither for the effort wrt to 
remembered set maintenance nor the effort it would maybe take to 
evacuate that extra region (compared to the amount of work before).

Obviously the mechanism is not tuned for maximum efficiency :)

Hth,
   Thomas