<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Chris,<br>
<br>
<div class="moz-cite-prefix">On 2/10/2016 3:53 PM, Chris Plummer
wrote:<br>
</div>
<blockquote cite="mid:56BBA353.2020805@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2/10/16 12:50 PM, Tom Benson
wrote:<br>
</div>
<blockquote cite="mid:56BBA27C.2080106@oracle.com" type="cite"> <br>
I've heard from another GC team person that there might be more
feedback on the name coming, after some discussion. Not sure if
it will constitute the 'landslide' I mentioned. 8^)<br>
</blockquote>
No problem. I'll wait for that to settle before sending out a
final webrev.<br>
</blockquote>
<br>
"Landslide" may not be the right term, but there was mild consensus
among the GC team that it's worth going through CCC again to replace
UseAggressiveHeapShrink. Our suggestion is "ShrinkHeapInSteps",
which would be on by default, and hopefully describes what's
happening without any other implications. So you'd disable it to
get what you want.<br>
Tom<br>
<br>
<blockquote cite="mid:56BBA353.2020805@oracle.com" type="cite"> <br>
thanks,<br>
<br>
Chris<br>
<blockquote cite="mid:56BBA27C.2080106@oracle.com" type="cite">
Tom<br>
<br>
<div class="moz-cite-prefix">On 2/10/2016 3:39 PM, Chris Plummer
wrote:<br>
</div>
<blockquote cite="mid:56BB9FEA.5070506@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Tom,<br>
<br>
if (!UseAggressiveHeapShrink) {<br>
// If UseAggressiveHeapShrink is false (the
default),<br>
// we don't want shrink all the way back to initSize
if people call<br>
// System.gc(), because some programs do that
between "phases" and then<br>
// we'd just have to grow the heap up again for the
next phase. So we<br>
// damp the shrinking: 0% on the first call, 10% on
the second call, 40%<br>
// on the third call, and 100% by the fourth call.
But if we recompute<br>
// size without shrinking, it goes back to 0%.<br>
shrink_bytes = shrink_bytes / 100 *
current_shrink_factor;<br>
}<br>
assert(shrink_bytes <= max_shrink_bytes, "invalid
shrink size");<br>
if (current_shrink_factor == 0) {<br>
_shrink_factor = 10;<br>
} else {<br>
_shrink_factor = MIN2(current_shrink_factor * 4,
(size_t) 100);<br>
}<br>
<br>
I got rid of the changes at the start of the method, and
added the !UseAggressiveHeapShrink check and the comment, so
the first 2 lines above and the closing right brace are now
the only change in the file, other than the copyright date.
If you want I could also move the _shrink_factor adjustment
into this block since the value of _shrink_factor becomes
irrelevant if UseAggressiveHeapShrink is true. The assert
should remain outside the block.<br>
<br>
cheers,<br>
<br>
Chris<br>
<br>
On 2/10/16 12:16 PM, Tom Benson wrote:<br>
</div>
<blockquote cite="mid:56BB9A9A.7060901@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi Chris,<br>
OK, that all sounds good. <br>
<br>
>> I can change it, although that will mean filing a
new CCC.<br>
Ah, I'd forgotten about that. Not worth it, unless there's
a landslide of support for a different name.<br>
<br>
Tnx,<br>
Tom<br>
<br>
<div class="moz-cite-prefix">On 2/10/2016 3:06 PM, Chris
Plummer wrote:<br>
</div>
<blockquote cite="mid:56BB9831.5030504@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Tom,<br>
<br>
Thanks for having a look. Comments inline below:<br>
<br>
On 2/10/16 11:27 AM, Tom Benson wrote:<br>
</div>
<blockquote cite="mid:56BB8F3D.3070502@oracle.com"
type="cite">Hi Chris, <br>
My apologies if I missed the discussion somewhere, but
is there a specific rationale for adding this that can
be mentioned in the bug report? I can imagine scenarios
where it would be useful, but maybe the real need can be
called out. <br>
</blockquote>
In general, it is for customers that want to minimize the
amount of memory used by the java heap, and are willing to
sacrifice some performance (induce more frequent GCs) to
save that memory. When heap usage fluctuates greatly, the
GC will tend to hold on to that memory longer than needed
due to the the current algorithm which requires 4 full GCs
before
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
MaxHeapFreeRatio is fully honored. If this is what you are
looking for, I can add it to the CR.<br>
<blockquote cite="mid:56BB8F3D.3070502@oracle.com"
type="cite"> <br>
I think it might be clearer if the new code in
cardGeneration was moved down to where the values are
used. IE, I would leave the inits of
current_shrink_factor and _shrink_factor as they were at
lines 190/191. Then down at 270, just don't divide by
the shrink factor if UseAggressiveHeapShrink is set, and
the updates to shrink factor can be in the same
conditional. This has the advantage that you can fix
the comment just above it to match this special case. Do
you think that would work? <br>
</blockquote>
Yes, that makes sense. I'll get started on it. I have a
vacation coming up shortly, so what I'll get a new webrev
out soon, but probably will need to wait until after my
trip to do more thorough testing and push the changes.<br>
<blockquote cite="mid:56BB8F3D.3070502@oracle.com"
type="cite"> <br>
It looks like the ending "\" at line 3330 in globals.hpp
isn't aligned, and the copyright in cardGeneration.cpp
needs to be updated. <br>
</blockquote>
Ok.<br>
<blockquote cite="mid:56BB8F3D.3070502@oracle.com"
type="cite"> <br>
One other nit, which you can ignore unless someone comes
forward to agree with me 8^) , is that I'd prefer the
name ShrinkHeapAggressively instead. Maybe this was
already debated elsewhere.... <br>
</blockquote>
The name choice hasn't really been discussed or
questioned. It was what was suggested to me, so I stuck
with it (The initial work was done by someone else. I'm
just getting it integrated into 9). I can change it,
although that will mean filing a new CCC.<br>
<br>
thanks,<br>
<br>
Chris<br>
<blockquote cite="mid:56BB8F3D.3070502@oracle.com"
type="cite">Tom <br>
<br>
On 2/4/2016 1:36 PM, Chris Plummer wrote: <br>
<blockquote type="cite">Hello, <br>
<br>
Please review the following for adding the -XX
UseAggressiveHeapShrink option. When turned on, it
tells the GC to reduce the heap size to the new target
size immediately after a full GC rather than doing it
progressively over 4 GCs. <br>
<br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecjplummer/8146436/webrev.02/">http://cr.openjdk.java.net/~cjplummer/8146436/webrev.02/</a>
<br>
Bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8146436">https://bugs.openjdk.java.net/browse/JDK-8146436</a>
<br>
<br>
Testing: <br>
-JPRT with '-testset hotspot' <br>
-JPRT with '-testset hotspot -vmflags
"-XX:+UseAggressiveHeapShrink"' <br>
-added new TestMaxMinHeapFreeRatioFlags.java test <br>
<br>
thanks, <br>
<br>
Chris <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>