<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
new webrev:<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/6521376/webrev.02/">http://cr.openjdk.java.net/~tamao/6521376/webrev.02/</a><br>
<br>
Please see inline.<br>
<br>
Thanks.<br>
Tao<br>
<br>
On 6/24/13 6:50 AM, Thomas Schatzl wrote:
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
Hi,
On Sat, 2013-06-22 at 20:03 -0700, Tao Mao wrote:
</pre>
<blockquote type="cite">
<pre wrap="">6521376: MaxTenuringThreshold and AlwayTenure/NeverTenure consistency
webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/6521376/webrev.00/">http://cr.openjdk.java.net/~tamao/6521376/webrev.00/</a>
</pre>
</blockquote>
<pre wrap="">
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.</pre>
</blockquote>
The most recent gc repo behavior:<br>
$java -XX:InitialTenuringThreshold=5 -XX:+AlwaysTenure -version<br>
<br>
runs without abort. It seems to "silently" accept the inconsistency
of ITT and MTT because the code doesn't sync MTT with AlwaysTenure
yet. Thus, it does<br>
<br>
In my changeset, with the sync of ITT and MTT, the behavior goes:<br>
<br>
$java -XX:+AlwaysTenure -XX:InitialTenuringThreshold=5 -version <br>
(or, $java -XX:InitialTenuringThreshold=5 -XX:+AlwaysTenure
-version)<br>
<br>
InitialTenuringThreshold of 5 is invalid; must be between 0 and 0<br>
Error: Could not create the Java Virtual Machine.<br>
Error: A fatal exception has occurred. Program will exit.<br>
<br>
This is the expected behavior so that we would warn the user change
ITT (or, simply remove it if s/he wishes to) to cope with
"-XX:+AlwaysTenure" (i.e. MTT=0).<br>
<br>
I don't see any need to sync ITT and MTT in other silent way.<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
- 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.
</pre>
</blockquote>
Fixed. Thank you for pointing out.<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
- 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.</pre>
</blockquote>
I'll write jtreg later once I consolidate the behaviors here.<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
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.</pre>
</blockquote>
I provide the expected behavior table below (mainly copied and
modified from the first email I sent)<br>
<br>
(1) Setting -XX:+NeverTenure/-XX:+AlwaysTenure
<br>
<br>
(1-a) -XX:+NeverTenure<br>
->
<br>
NeverTenure=true; AlwaysTenure=false;
MaxTenuringThreshold=markOopDesc::max_age+1 (i.e. 16, use the value
instead below);
<br>
<br>
(1-b) -XX:+AlwaysTenure<br>
->
<br>
NeverTenure=false; AlwaysTenure=true; MaxTenuringThreshold=0;
<br>
<br>
(2) Setting MaxTenuringThreshold
<br>
<br>
(2-a) MaxTenuringThreshold == 0
<br>
-><br>
NeverTenure=false; AlwaysTenure=true;<br>
<br>
(2-b) 0 < MaxTenuringThreshold < 16<br>
-><br>
NeverTenure=false; AlwaysTenure=false;<br>
<br>
(2-c) MaxTenuringThreshold = 16<br>
-><br>
NeverTenure=true; AlwaysTenure=false;<br>
<br>
(2-d) MaxTenuringThreshold > 16<br>
-><br>
VM should abort due to the check<br>
<meta charset="utf-8">
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">2164 status = status && verify_percentage(YoungGenerationSizeSupplement, "YoungGenerationSizeSupplement");
2165 status = status && verify_percentage(TenuredGenerationSizeSupplement, "TenuredGenerationSizeSupplement");
2166
<b>2167 status = status && verify_interval(MaxTenuringThreshold, 0, markOopDesc::max_age + 1, "MaxTenuringThreshold");</b>
2168 status = status && verify_interval(InitialTenuringThreshold, 0, MaxTenuringThreshold, "InitialTenuringThreshold");
2169 status = status && verify_percentage(TargetSurvivorRatio, "TargetSurvivorRatio");</pre>
<br>
*(3) -XX:-NeverTenure/-XX:-AlwaysTenure only need to take care of
themselves and don't need the flag consistency maintenance here.
<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">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.
</pre>
</blockquote>
<pre wrap="">
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.</pre>
</blockquote>
See the behavior summary above.<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
Maybe others can comment on the expected behavior better than me.
</pre>
<blockquote type="cite">
<pre wrap="">*(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.
*******************************************************************************
</pre>
</blockquote>
<pre wrap="">
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...
</pre>
</blockquote>
I agree.<br>
<blockquote cite="mid:1372081854.2721.138.camel@cirrus" type="cite">
<pre wrap="">
Thanks,
Thomas
</pre>
</blockquote>
</body>
</html>