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

[Jon Masamitsu]

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".

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
>>>>>>>
>>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130305/5edf4eb4/attachment.htm>