8262185: Some questions regarding prune collection set candidates early

[Tianqi Xia]

Hi Thomas,

Thanks for sharing these insights. The alternative you mentioned sounds
interesting and I will keep investigating.

I agree that the impact of JDK-8286467 should be minimal, if any, but I am
happy to make a fix for that :)

Regards,
Tianqi

On Tue, May 10, 2022 at 5:32 PM Thomas Schatzl <thomas.schatzl at oracle.com>
wrote:

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