Request for review: 8007053: Refactor SizePolicy code for consistency across collectors

[Bengt Rutisson]

Hi Tao,

I have only looked briefly at your webrev, but I have a request before I 
look more at it.

Could you split it up into a few different changes? I find it difficult 
get a grasp of the changes when there are so many changed files and they 
contain changes for different reasons.

You probably know better than me how to split this up. But I think I 
would like to have at least these three separate changes:

* Renaming of methods
* Splitting up compute_generations_free_space()
* Changes to use MaxGCPauseMillis instead of MaxGCMinorPauseMillis

If this division does not make sense to you I have probably drawn the 
wrong conclusions. But we need to split the review up in some way to 
make it easier to review. I'm fine with other ways of splitting it up if 
you find more natural ways of splitting it up.

With splitting up the changes I mean filing separate bugs and submitting 
separate webrevs, so that they can be pushed as separate changesets.

Also, I don't think you should include the white space changes to these 
two files:
psGCAdaptivePolicyCounters.hpp
adaptiveSizePolicy.hpp

Thanks,
Bengt


On 1/31/13 1:37 AM, Tao Mao wrote:
> 8007053: Refactor SizePolicy code for consistency across collectors
> https://jbs.oracle.com/bugs/browse/JDK-8007053
>
> webrev:
> http://cr.openjdk.java.net/~tamao/8007053/webrev.00/
>
> changeset:
> 1. rename a bunch of functions across collectors related to compute 
> and resize young/tenured gens
> unified function names:
> (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()
>
> 2. split compute_generations_free_space() into two functions:
> compute_eden_space_size() + compute_tenured_generation_free_space()
> each of which (if needed) can be reused without executing an overhead 
> of the other.
>
> *3. in src/share/vm/memory/collectorPolicy.cpp
> a minor bug in initializing an AdaptiveSizePolicy instance is caught.
> MaxGCMinorPauseMillis -> MaxGCPauseMillis
>
> testing:
> refworkload test cases: jetstream, scimark, specjbb2000, specjbb2005, 
> specjvm98
> GC options: -XX:+UseParallelGC -Xmx512m
> no regression found from the performance results;
> PrintGCStats and CompareGCStats show no abnormal variations.
>