RFR: 8138975: G1CollectorPolicy::calculate_young_list_target_length should be const

[Mikael Gerdin]

Hi Erik,

On 2015-10-06 17:28, Erik Helin wrote:
> Hi all,
>
> this patch splits G1CollectorPolicy::update_young_list_target_length into
> two methods:
> - void G1CollectorPolicy::update_young_list_target_length()
> - size_t
>    G1CollectorPolicy::bounded_young_list_target_length(bool is_young, size_t rs_lenghts) const
>
> This enables bounded_young_list_target_length to be const and then
> update_young_list_target_length can just set the field
> _young_list_target_length to the newly calculated value. Making
> bounded_young_list_target_length const revaled some strange
> side effects of update_young_list_target_length:
> - update_young_list_target_length also called
>    update_max_gc_locker_expansion. I moved the call to
>    update_max_gc_locker_expansion to all callsites of
>    update_young_list_target_length.

I'm not sure I like moving the call to update_max_gc_locker_expansions 
to all callers of update_young_list_target_length.

Would you mind adding a
void update_young_list_lengths() {
update_young_list_target_length();
update_max_gc_locker_expansion();
}
to encapsulate the code modifying the young list length a little?


> - update_young_list_target_length sometimes updated the field
>    _rs_lenghts_prediction. This is only needed when
>    update_young_list_target_length is called from the
>    ConcurrentG1RefinementThread that is scanning the rembered sets sizes
>    of the YoungList, because _rs_lenghts_prediction is state that belongs
>    to that thread but for some reason ended up in G1CollectorPolicy.
>    Anyways, I moved the update of the field to only place where it is
>    needed. This move will actually cause the "// Add 10% to avoid having to
>    recalculate too often" comment to become correct since previously the
>    field was only set via some code paths in
>    update_young_list_target_length.

Now that I've thought about this a bit I disagree. I think we need the 
code in record*collection*end to update the _rs_lenghts_prediction.
Otherwise we could end up in a scenario where the condition in 
revise_young_list_target_length_if_necessary is never true since the 
sampled lengths is much smaller than the previous collection's prediction.

Otherwise _rs_lenghts_prediction will degenerate to the maximum 
prediction ever performed during the lifetime of a process.

>
> Enhancement:
> https://bugs.openjdk.java.net/browse/JDK-8138975
>
> Webrev:
> http://cr.openjdk.java.net/~ehelin/8138975/webrev/

I discussed the is_young bool parameter with Erik and we settled on 
removing it at this point.
A lot of the underlying code still looks at the collector_state instead 
of getting young/mixed via a parameter so we'll leave that as is for now.

/Mikael

>
> Testing:
> JPRT
>
> Thanks,
> Erik
>