<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~cjplummer/8146436/webrev.03/">http://cr.openjdk.java.net/~cjplummer/8146436/webrev.03/</a><br>
    <br>
    thanks,<br>
    <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>
  </body>
</html>