Request for review: 6521376: MaxTenuringThreshold and AlwayTenure/NeverTenure consistency

Thomas Schatzl thomas.schatzl at oracle.com
Mon Jun 24 13:50:54 UTC 2013


Hi,

On Sat, 2013-06-22 at 20:03 -0700, Tao Mao wrote:
> 6521376: MaxTenuringThreshold and AlwayTenure/NeverTenure consistency
> 
> webrev:
> http://cr.openjdk.java.net/~tamao/6521376/webrev.00/

The webrev includes a complete psScavenge.cpp. To get to the actual
changes, you have to manually diff it. Could you please minimize the
change for that file and re-upload the webrev?

I manually diffed that files with the current version to find the
actual changes...

Some notes:

- the change does not maintain consistency of MaxTenuringThreshold with
the InitialTenuringThreshold variable.
Running the VM with -XX:InitialTenuringThreshold=5 -XX:+AlwaysTenure
will give an error (note that InitialTT is set before AlwaysTenure); the
other way the VM should give an error.

- the change in AgeTable::compute_tenuring_threshold() effectively also
disables the output of PrintTenuringDistribution and updates related to
UsePerfData. Particularly the latter seems to be a bug.

- please provide test cases that cover all possible classes of
combinations for these variables (Initial/MaxTenuringThreshold,
Always/NeverTenure). You could use the test case for 8014765 as basis.

There are already some helper classes in the test/gc/arguments directory
that e.g. allow you to parse the output of -XX:+PrintFlagsFinal to verify
them.

It may help (also us) to also provide one or more small tables with expected behavior given input values, and use that to catch all possible situations.

> changeset: For all collectors,
> 
> (1) Setting -XX:+NeverTenure/-XX:+AlwaysTenure
> "-XX:+NeverTenure"
> ->
> NeverTenure=true; AlwaysTenure=false;  
> MaxTenuringThreshold=markOopDesc::max_age+1 (i.e. 16, use the value 
> instead below);
> 
> "-XX:+AlwaysTenure"
> ->
> NeverTenure=false; AlwaysTenure=true;  MaxTenuringThreshold=0;
> 
> (2) Setting MaxTenuringThreshold
> 
> (2-a) MaxTenuringThreshold == 0
> if (MaxTenuringThreshold == 0):
> "the cap equals 0" implies AlwaysTenure=true, so set flags accordingly 
> (NeverTenure=false; AlwaysTenure=true).
> 
> (2-b) MaxTenuringThreshold > 0
> But, considering the ergonomics for tenuring threshold (see 
> PSAdaptiveSizePolicy::compute_survivor_space_size_and_threshold(), 
> CMSAdaptiveSizePolicy::compute_survivor_space_size_and_threshold() and 
> ageTable::compute_tenuring_threshold()), we will adjust tenure_threshold 
> up and down but below the cap MaxTenuringThreshold during the 
> application run.
> 
> Thus, setting MaxTenuringThreshold >= 16 would not necessarily imply 
> that the user wants to apply NeverTenure to the VM for the real 
> tenure_threshold may need to go below MaxTenuringThreshold as the 
> process stats suggest.
>
> With that said,
> if (MaxTenuringThreshold > 0):
> I simply set NeverTenure/AlwaysTenure to false. (neither NeverTenure nor 
> NeverTenure)
> and if (MaxTenuringThreshold > 16): set MaxTenuringThreshold to 16 as it 
> is unnecessary to be larger.

Some notes:

0 <= MaxTenuringThreshold < 16:
Behavior as before, seems okay.

MaxTenuringThreshold >= 16:
This is a change to previous behavior. Previously the VM would have
simply exited, telling the user that he passed an illegal value, while
the VM now silently sets the value to 16.

Looking at the CR which gives some example about the use, a value of 16
may be useful, i.e. never tenure for a while.

Both compute_survivor_space_size_and_threshold() methods also change the
tenuring thresholds incrementally: so a tenuring threshold > 16 may be
useful (I do not know). E.g. to keep the tenuring threshold high during
startup.

I am not really convinced that setting a tenuring threshold >= 16 makes
a lot of sense, but I cannot find a reason why 16 is better than any
other higher value, i.e. the reason for silently setting everything above
that value to 16.

Maybe others can comment on the expected behavior better than me.

> *(3) -XX:-NeverTenure/-XX:-AlwaysTenure only need to take care of 
> themselves and don't need the flag consistency maintenance here.
> 
> *******************************************************************************
> Take a little convoluted case for example: suppose that we have the 
> following gc options "-XX:+NeverTenure -XX:+MaxTenuringThreshold=18" in 
> this particular order.
> 
> When we first parse "-XX:+NeverTenure", we would set: NeverTenure=true; 
> AlwaysTenure=false;  MaxTenuringThreshold=16.
> 
> But when we encounter the second option "-XX:+MaxTenuringThreshold=18", 
> we would think the user knows about gc ergonomics for tenuring threshold 
> and wants MaxTenuringThreshold to just be a cap for the ergonomics 
> rather than wants to (implicitly) use NeverTenure. So, we would set 
> NeverTenure=false; AlwaysTenure=false; and finally 
> MaxTenuringThreshold=16 to reflect the cap maximum.
> *******************************************************************************

Instead of guessing the user's intent it may sometimes be better to just
tell the user that the given options do not make sense imo...

Thanks,
Thomas






More information about the hotspot-gc-dev mailing list