Request for review: 8007764: Wrong initialized value of max_gc_pause_sec for an instance of class AdaptiveSizePolicy

Tao Mao tao.mao at
Fri Feb 8 23:44:12 UTC 2013

I'm trying to answer these questions. Please see inline. Thanks.

On 2/8/13 8:17 AM, Bengt Rutisson wrote:
> Hi Tao,
> I think the change looks good and makes sense.
> But I have a hard time estimating the implications of this change.
> It seems like MaxGCMinorPauseMillis is only used by CMS and 
> ParallelScavenge. 
You are right. Before the fix, four occurrences below. (The one in 
collectorPolicy.cpp is suspected to be wrong)

MaxGCMinorPauseMillis is introduced in different kinds of classes for 
CMS and PS: ConcurrentMarkSweep*Policy* and ParalleScavenge*Heap*.

$ grep -nr "MaxGCMinorPauseMillis" 
double max_gc_minor_pause_sec = ((double) MaxGCMinorPauseMillis)/1000.0;
double max_gc_minor_pause_sec = ((double) MaxGCMinorPauseMillis)/1000.0;
src/share/vm/memory/collectorPolicy.cpp:170:  const double 
max_gc_minor_pause_sec = ((double) MaxGCMinorPauseMillis)/1000.0;
src/share/vm/runtime/globals.hpp:2043:  product(uintx, 
MaxGCMinorPauseMillis, max_uintx,                          \
> For both of those the default for MaxGCMinorPauseMillis is the same as 
> MaxGCPauseMillis so it should be fine.
How did you infer that the default value for MaxGCMinorPauseMillis is 
same as MaxGCPauseMillis? I didn't see such an indication in the code.
> But what happens if a customer is running with 
> -XX:MaxGCMinorPauseMillis=1000 but not setting MaxGCPauseMillis on the 
> command line? What are the implications of your change for such runs? 
> I don't think it is very common. Just thinking.
For ParallelScavenge, customers should distinguish the two flags.
For CMS, I wonder whether we have fully implemented adaptive sizing 
policy. Jon, can you comment on this?
In either case, customers should be able to tell apart.
> Also, a more common case is probably to run with only 
> -XX:MaxGCPauseMillis set on the command line. How will the adaptive 
> sizing behave differently compared to before?
The fix only changed the value in the routine 
GenCollectorPolicy::initialize_size_policy, which is believed to be 
called within
the routine of GenCollectedHeap::post_initialize(). I'm not sure whether 
this routine can change sizing behaviors somewhere.
> Looking closer at the code it looks like CMS and ParallelScavenge are 
> actually using CMSAdaptiveSizePolicy and PSAdaptiveSizePolicy 
> respectively. Both of these already pass the correct value to the 
> constructor of the super class AdaptiveSizePolicy. So, this bug is 
> currently benign. The code that you are changing is actually not being 
> used. Is this a correct conclusion?
See above. Whether the code I'm changing is being used depends upon 
whether GenCollectedHeap::post_initialize() is called anywhere. But I 
can't determine that right now.

Other than that, the fix should correct the bug while preserving 
everything else.
> Thanks,
> Bengt
> On 2/8/13 3:42 AM, Tao Mao wrote:
>> 8007764: Wrong initialized value of max_gc_pause_sec for an instance 
>> of class AdaptiveSizePolicy
>> webrev:
>> changeset:

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the hotspot-gc-dev mailing list