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

[Tao Mao]

Thank you all for looking at this code.

After JDK-8007763, it's easier to validate this refactoring.

Please review the new webrev.
http://cr.openjdk.java.net/~tamao/8007762/webrev.03/

Thanks
Tao

On 5/15/13 4:51 PM, John Cuthbertson wrote:
> 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
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>>
>>>
>