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

[Bengt Rutisson]

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/04c05c80/attachment.htm>