Request for review: 8007762: Rename a bunch of methods in size policy across collectors

[John Cuthbertson]

Hi Tao,

adaptiveSizePolicy.cpp:470
Shouldn't the name of the routine in the print be 
AdaptiveSizePolicy::check_gc_overhead_limit?

Other than that, the changes look OK to me.

JohnC

On 5/6/2013 2:49 PM, Tao Mao wrote:
> JDK-8007762 and JDK-8007763 are coupled as sort of "chicken-egg" problem.
>
> Reviewers for this webrev, please review JDK-8007763 first. (Search 
> mails for "Request for review: 8007763")
>
> Thanks.
> Tao
>
> On 3/5/13 8:46 PM, Jon Masamitsu wrote:
>> Tao,
>>
>> Bengt was ok with leaving the name compute_generation_free_space()
>> as is so I think we should do that.   Bengt's point about methods
>> needing better names probably applies to several methods and
>> I'd rather consider a renaming scheme after your refactoring
>> changes have been integrated.
>>
>> Jon
>>
>> On 3/5/2013 10:57 AM, Bengt Rutisson wrote:
>>>
>>> Jon,
>>>
>>> On 3/5/13 5:31 PM, Jon Masamitsu wrote:
>>>> Bengt,
>>>>
>>>> Would a less specific name such as "update" be
>>>> better?  Something that says to the reader
>>>> "you have to go look at what the method is
>>>> doing because a name is not going to be
>>>> descriptive enough".
>>>
>>> Yes, I think so. Something that indicates that this will have side 
>>> effects. In my earlier email I suggested 
>>> update_adaptive_size_values(). If you come up with something better 
>>> I'm happy too. :)
>>>
>>> Bengt
>>>
>>>>
>>>> Jon
>>>>
>>>> On 03/04/13 22:56, Bengt Rutisson wrote:
>>>>>
>>>>> Hi Tao,
>>>>>
>>>>> The method PSAdaptiveSizePolicy::compute_generation_free_space() 
>>>>> is a monster method. It does all kinds of things. Its comment kind 
>>>>> of reveals this:
>>>>>
>>>>>  339   // Calculates optimial free space sizes for both the old 
>>>>> and young
>>>>>  340   // generations.  Stores results in _eden_size and _promo_size.
>>>>>  341   // Takes current used space in all generations as input, as 
>>>>> well
>>>>>  342   // as an indication if a full gc has just been performed, 
>>>>> for use
>>>>>  343   // in deciding if an OOM error should be thrown.
>>>>>
>>>>> So, I think that if we should rename it we should give it a name 
>>>>> that reflects this. The current name is not good, but I don't 
>>>>> think compute_generations_free_space() is much better. I would 
>>>>> suggest to either leave it unchanged or change to a more 
>>>>> descriptive name.
>>>>>
>>>>> Maybe gether_adapitve_size_statistics() or 
>>>>> update_adaptive_size_values(). Not very happy with those names 
>>>>> either, but at least they indicate that ther eis more than just 
>>>>> free space calculations in there.
>>>>>
>>>>>
>>>>> In psGCAdaptivePolicyCounters.hpp you updated a method name that 
>>>>> has been commented out. I would suggest removing it rather than 
>>>>> updating it.
>>>>>
>>>>> Thanks,
>>>>> Bengt
>>>>>
>>>>> On 3/5/13 1:46 AM, Tao Mao wrote:
>>>>>> Oops...sent to open list.
>>>>>>
>>>>>> Thanks.
>>>>>> Tao
>>>>>>
>>>>>> On 3/4/2013 4:38 PM, Tao Mao wrote:
>>>>>>> This is the proposed final webrev, with copyright dated 
>>>>>>> appropriately.
>>>>>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.02/
>>>>>>>
>>>>>>> Jon,
>>>>>>> A patch is rendered below. Please take the patch and help push 
>>>>>>> the changeset.
>>>>>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.02/8007762RenameMethodsInSizePolicy.patch 
>>>>>>>
>>>>>>>
>>>>>>> Thank you.
>>>>>>> Tao
>>>>>>>
>>>>>>> On 2/26/13 8:37 AM, Jon Masamitsu wrote:
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>> Looks good.
>>>>>>>>
>>>>>>>> Jon
>>>>>>>>
>>>>>>>> On 02/25/13 11:09, Tao Mao wrote:
>>>>>>>>> A new webrev is updated.
>>>>>>>>> Thanks.
>>>>>>>>> Tao
>>>>>>>>>
>>>>>>>>> On 2/18/2013 7:46 AM, Jon Masamitsu wrote:
>>>>>>>>>> Tao,
>>>>>>>>>>
>>>>>>>>>> Why this change from resize_old_gen() to
>>>>>>>>>> resize_tenured_gen()?  There are many
>>>>>>>>>> variable names and comments referring to
>>>>>>>>>> "old gen" still in ParallelScavengeHeap.
>>>>>>>>> Reverted.
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.00/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp.udiff.html 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Jon
>>>>>>>>>>
>>>>>>>>>> On 02/14/13 11:18, Tao Mao wrote:
>>>>>>>>>>> 8007762: Rename a bunch of methods in size policy across 
>>>>>>>>>>> collectors
>>>>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8007762
>>>>>>>>>>>
>>>>>>>>>>> webrev:
>>>>>>>>>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> changeset:
>>>>>>>>>>> The motivation of this change is associated with CR 7098155 
>>>>>>>>>>> (https://jbs.oracle.com/bugs/browse/JDK-7098155). For this, 
>>>>>>>>>>> we need to split compute_generations_free_space into two 
>>>>>>>>>>> routine (one for compute_eden_space_size and the other for 
>>>>>>>>>>> compute_tenured_generation_free_space) in order to save some 
>>>>>>>>>>> overhead when we want to compute only one of the two. The 
>>>>>>>>>>> next step is to split 
>>>>>>>>>>> PSAdaptiveSizePolicy::compute_generations_free_space(), 
>>>>>>>>>>> which will be resolved in CR 8007763.
>>>>>>>>>>>
>>>>>>>>>>> To unify the naming, a bunch of routines are renamed here.
>>>>>>>>>>> (1) compute_*:
>>>>>>>>>>> compute_survivor_space_size_and_threshold()
>>>>>>>>>>>       compute_generations_free_space() = 
>>>>>>>>>>> compute_eden_space_size() + 
>>>>>>>>>>> compute_tenured_generation_free_space()
>>>>>>>>>>> (2) resize_*_gen:
>>>>>>>>>>>       resize_young_gen()
>>>>>>>>>>>       resize_tenured_gen()
>>>>>>>>>>> (3) rename judging routine resize_geneneration() to 
>>>>>>>>>>> need_resize() to avoid ambiguity
>>>>>>>>>>>
>>>>>>>>>>> testing:
>>>>>>>>>>> purely renaming; passed JPRT
>>>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>