<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<br>
<div class="moz-cite-prefix">On 2/29/2016 10:10 PM, Chris Plummer
wrote:<br>
</div>
<blockquote cite="mid:56D5325B.80903@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2/25/16 10:23 AM, Chris Plummer
wrote:<br>
</div>
<blockquote cite="mid:56CF46B4.9080800@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2/22/16 8:19 AM, Chris Plummer
wrote:<br>
</div>
<blockquote cite="mid:56CB3524.7030800@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 2/18/16 7:21 AM, Tom Benson
wrote:<br>
</div>
<blockquote cite="mid:56C5E177.9030104@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
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>
</blockquote>
Hi Tom,<br>
<br>
Ok, I'll get started with this. Just got back from vacation
today, but it shouldn't take long to get to it.<br>
<br>
</blockquote>
Updated webrev:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecjplummer/8146436/webrev.03/">http://cr.openjdk.java.net/~cjplummer/8146436/webrev.03/</a><br>
<br>
thanks,<br>
</blockquote>
Ping. Tom and Jon can you give final approval for this?<br>
</blockquote>
<br>
Changes look good. Reviewed.<br>
<br>
Jon<br>
<br>
<blockquote cite="mid:56D5325B.80903@oracle.com" type="cite"> <br>
Thanks.<br>
<br>
Chris<br>
<blockquote cite="mid:56CF46B4.9080800@oracle.com" type="cite"> <br>
Chris<br>
<br>
<blockquote cite="mid:56CB3524.7030800@oracle.com" type="cite">
thanks,<br>
<br>
Chris<br>
<blockquote cite="mid:56C5E177.9030104@oracle.com" type="cite">
<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>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>