RFR (XXS): 8138752: G1CollectorPolicy::should_should_update_surv_rate_group_predictors() uses wrong predicate

[Derek White]

On 10/2/15 8:54 AM, Thomas Schatzl wrote:
> Hi all,
>
>    can I have reviews for this very-small fix for a bug introduced in
> JDK-7097567?
>
> Instead of using
>
> CollectorState::last_young_gc() && !CollectorState::in_marking_window()
>
> to determine when to update the survivor rate group predictors (which
> means "only the gc between GC cleanup and mixed GC"), G1 needs to do
> this for all young GCs using
>
> CollectorState::last_gc_was_young() && !
> CollectorState::in_marking_window()
>
> which means just during young-only GCs outside of marking.
>
> The original code in JDK-7097567 looks like
>
>
> -    bool propagate = _last_gc_was_young && !_in_marking_window;
> +    bool propagate = collector_state()->should_propagate();
>
> (http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorPolicy.hpp)
>
> where CollectorState::should_propagate() has been wrongly changed to:
>
> bool should_propagate() { // XXX should have a more suitable state name or abstraction for this
>     return (_last_young_gc && !_in_marking_window);
> }
Ahh, when I ported Ramki's original patch he had:
return (_last_young_gc_full && !_in_marking_window);

But the variable "_last_young_gc_full" no longer exists. I miss-updated 
that to "_last_young_gc", and didn't catch it in the diff.

I really need to rename these variables like I've been threatening to. 
They are very confusing!

(r)eviewed.
> (http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorState.hpp)
>
> It is based on the changes for 8138750 which is also currently out for
> review.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8138752
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8138752/webrev/
> Testing:
> local compilation
>
> Thanks,
>    Thomas
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20151008/66a018ed/attachment.htm>