RFR(L) 8154154: Separate G1 specific policy code from the CollectorPolicy class hierarchy

[Mikael Gerdin]

Hi Kim,

On 2016-04-15 19:27, Kim Barrett wrote:
>> On Apr 15, 2016, at 3:37 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>> On 2016-04-15 01:06, Kim Barrett wrote:
>>>> On Apr 14, 2016, at 7:23 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>>>> Testing: RBT GC Testing, JPRT, Performance testing on aurora.
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8154154
>>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8154154/webrev.0/
>>>
>>> OK, this review was *much* easier than I expected.
>>>
>>> Looks good, with just a few very minor comments.
>>
>> Thanks.
>> I'd like to pick up on your offer of deferring the cleanups to new RFEs since this change feels large enough as-is and I've tried to avoid changing too much of the semantics in this change.
>
> OK.
>
>>> src/share/vm/gc/g1/concurrentG1Refine.cpp
>>>    30 #include "gc/g1/g1Policy.hpp"
>>>    59   G1Policy* policy = g1h->g1_policy();
>>>
>>> These changes will be obsoleted by my in-review changes for 8133051.
>>> One of use is going to have a (minor) merge collision to deal with.
>>
>> Yep, I don't mind taking the merge hit since your change already has a bunch of reviews.
>
> Well, you have two completed reviews, where I think I only have one so far.
> But the repos are closed right now anyway.  I don’t expect it to be a big deal
> for whichever of us goes second, so long as we’re not both trying to push at
> the same time and the second gets bounced for merge conflicts.

In that case I'll go ahead and push this.

>
>>> src/share/vm/gc/g1/g1YoungGenSizer.hpp
>>>    88   // Calculate the maximum length of the young gen given the number of regions
>>>    89   // depending on the sizing algorithm.
>>>    90   void compute_max_new_size(uint number_of_heap_regions);
>>>
>>> A name of calculate_xxx suggests to me that it computes and returns a
>>> value.  This function is called for side effect, e.g. it updates
>>> MaxNewSize.
>>
>> I agree, how about record_max_new_size?
>
> adjust_max_new_size is my current favorite, but you decide.  update_xxx is another option.

In that case I'll go for adjust, thanks for the review.

/Mikael

>
>