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

[Mikael Gerdin]

Hi Thomas,

On 2015-10-02 14:54, 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);
> }
>
> (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/

Both of us reviewed the incorrect change, let's hope we do better this 
time :)

Looks good.
/Mikael

> Testing:
> local compilation
>
> Thanks,
>    Thomas
>
>