<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    Ramki,<br>
    <br>
    Thanks for the extra testing!<br>
    <br>
    Jon, John and Tony,<br>
    <br>
    Thanks for the reviews! Are you OK with pushing this (handle values
    < 1 as never force full compaction):<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/">http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/</a><br>
    <br>
    Instead of this (forbid values < 1):<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/7110718/webrev.02/">http://cr.openjdk.java.net/~brutisso/7110718/webrev.02/</a><br>
    <br>
    I think your review comments were more related to webrev.02, so I
    would like to double check with you before I push my changes in
    webrev.03.<br>
    <br>
    Thanks,<br>
    Bengt<br>
    <br>
    <br>
    On 2011-11-18 08:11, Srinivas Ramakrishna wrote:
    <blockquote
cite="mid:CABzyjymetvJfuZF_bTAh_sTc3Kc8BrfQmfUDRTTwCVBtxyFPmw@mail.gmail.com"
      type="cite">Hi Bengt -- this looks good to me. It also passed my
      testing...<br>
      I also confirmed what Jon indicated -- that VerifyUpdateClosure is
      dead code, as far as my cscope<br>
      navigation showed and can be deleted (in a separate CR as Jon
      said).<br>
      <br>
      Rebiewed; thanks!<br>
      -- ramki<br>
      <br>
      <div class="gmail_quote">On Thu, Nov 17, 2011 at 6:13 AM, Bengt
        Rutisson <span dir="ltr"><<a moz-do-not-send="true"
            href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>></span>
        wrote:<br>
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex;">
          <div bgcolor="#ffffff" text="#000000"> <br>
            Hi Ramki,<br>
            <br>
            Is this what you were considering?<br>
            <a moz-do-not-send="true"
              href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.03/"
              target="_blank">http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/</a><br>
            <br>
            I think it should interpret MarkSweepAlwaysCompactCount
            <= 0 as "never force full compaction". It would be great
            if you could do some testing. There is one assert in
            PSParallelCompact::VerifyUpdateClosure::do_addr() that
            worries me a bit:<br>
            <br>
              assert(HeapMaximumCompactionInterval > 1 ||
            MarkSweepAlwaysCompactCount > 1 ||<br>
                     forwarding_ptr == new_pointer, "new location is
            incorrect");<br>
            <br>
            I think this should be safe since
            MarkSweepAlwaysCompactCount == 1 was an acceptable value
            even before my change. But I have to admit that I don't
            really understand what the assert is trying to verify.<br>
            <font color="#888888"> <br>
              Bengt</font>
            <div>
              <div class="h5"><br>
                <br>
                <br>
                On 2011-11-16 19:01, Srinivas Ramakrishna wrote:
                <blockquote type="cite">Hi Bengt, Not sure how much
                  customers use this option. Its useful for
                  "serviceability in production" kind of scenarios<br>
                  to have the code be more robust. I think it would be
                  useful. I appreciate the need for more testing<br>
                  of course, and I am happy to do that testing for you
                  -- just let me know and I'll grab yr patch and test<br>
                  here.<br>
                  <br>
                  thanks!<br>
                  -- ramki<br>
                  <br>
                  <div class="gmail_quote">On Tue, Nov 15, 2011 at 6:50
                    AM, Bengt Rutisson <span dir="ltr"><<a
                        moz-do-not-send="true"
                        href="mailto:bengt.rutisson@oracle.com"
                        target="_blank">bengt.rutisson@oracle.com</a>></span>
                    wrote:<br>
                    <blockquote class="gmail_quote" style="margin:0pt
                      0pt 0pt 0.8ex;border-left:1px solid
                      rgb(204,204,204);padding-left:1ex">
                      <div bgcolor="#FFFFFF" text="#000000"> <br>
                        Ramki,
                        <div><br>
                          <br>
                          On 2011-11-14 20:32, Srinivas Ramakrishna
                          wrote:
                          <blockquote type="cite"><br>
                            Thanks, Bengt, for the super-quick
                            turnaround!! A comment below on the choice
                            of <= 0 for the option value....<br>
                          </blockquote>
                          <br>
                        </div>
                        Thanks for the review! See comments below.
                        <div><br>
                          <br>
                          <blockquote type="cite">
                            <div class="gmail_quote">On Mon, Nov 14,
                              2011 at 1:25 AM, Bengt Rutisson <span
                                dir="ltr"><<a moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com" target="_blank">bengt.rutisson@oracle.com</a>></span>
                              wrote:<br>
                              <blockquote class="gmail_quote"
                                style="margin:0pt 0pt 0pt
                                0.8ex;border-left:1px solid
                                rgb(204,204,204);padding-left:1ex"><br>
                                Hi all,<br>
                                <br>
                                Can I have a couple of reviews for this
                                small change?<br>
                                <a moz-do-not-send="true"
                                  href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.01/"
                                  target="_blank">http://cr.openjdk.java.net/~brutisso/7110718/webrev.01/</a><br>
                                <br>
                                It is a fix for the issue that Ramki
                                reported recently.
                                MarkSweepAlwaysCompactCount is used for
                                division and Hotspot crashes if it is
                                set to 0.<br>
                                <br>
                                I choose to log an error and exit the VM
                                if someone tries to start with
                                -XX:MarkSweepAlwaysCompactCount=0. An
                                alternative is to just log a warning and
                                set it to 1.<br>
                                <br>
                                I prefer the error way since it is not
                                really clear what one wants to achieve
                                with MarkSweepAlwaysCompactCount=0.
                                Always do full compactions or never do
                                full compactions? So I am not convinced
                                that 1 is an appropriate value.<br>
                                <br>
                                Also, since the VM, up until now, has
                                crashed if someone tried
                                -XX:MarkSweepAlwaysCompactCount=0 I
                                think we can be sure that there are no
                                customers that are running with that
                                setting. It should be safe to forbid it.<br>
                              </blockquote>
                              <div><br>
                                I agree with that statement. However,
                                given that the value 0 was producing
                                crashes, proving that no production code
                                would have been using that setting,<br>
                                and based on yr comment above that the 0
                                value could as well have been used to
                                denote "never force full compaction", it
                                seems as though an alternative to
                                exiting with<br>
                                an error, is now to define all values
                                <=0 to mean "never _force_ full
                                compaction" <br>
                                <br>
                                Especially since tolerating allowed
                                input values and mapping them to
                                specific non-exiting behaviours allows
                                us to modify production JVM's on the fly<br>
                                without causing loss of availability.
                                (Consider a future in which this option
                                becomes a "manageable"; you would then
                                be faced with the same<br>
                                question, and it seems as though making
                                this choice now would help maintain
                                consistency and robustness going forward
                                -- we could of course<br>
                                always throw a "illegal value exception"
                                or such at that point, but allowing the
                                specification of "never _force_ full
                                compaction" (unless the JVM<br>
                                otherwise chooses to) would appear to be
                                a choice to allow users; mapping
                                negative and 0 values to that setting
                                would avoid having to<br>
                                throw an error.) However, I understand
                                that this is somewhat subjective, so I
                                am willing to go with whatever the
                                majority consensus here<br>
                                mght be. It just seemed more pleasant
                                to:<br>
                                (1) allow the specification of
                                reasonable behaviour (i.e. never _force_
                                ...)<br>
                                (2) map the full domain of the option to
                                a reasonable behaviour (i.e. allow <=
                                0 to map to never _force_ ..)<br>
                                <br>
                                Comments?<br>
                              </div>
                            </div>
                          </blockquote>
                          <br>
                        </div>
                        I see your point, and I think this should be
                        fairly straight forward to fix. However it will
                        require some more testing etc. I can do that,
                        but I don't think I know enough to say whether
                        or not the extra work is worth it. How important
                        is this option? Is it something that customers
                        use a lot?<br>
                        <br>
                        Thanks,<br>
                        <font color="#888888"> Bengt</font>
                        <div><br>
                          <br>
                          <blockquote type="cite">
                            <div class="gmail_quote">
                              <div><br>
                                -- ramki<br>
                                <br>
                                 <br>
                              </div>
                              <blockquote class="gmail_quote"
                                style="margin:0pt 0pt 0pt
                                0.8ex;border-left:1px solid
                                rgb(204,204,204);padding-left:1ex"> <br>
                                CR:<br>
                                <br>
                                7110718
                                -XX:MarkSweepAlwaysCompactCount=0
                                crashes the JVM<br>
                                <a moz-do-not-send="true"
                                  href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110718"
                                  target="_blank">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110718</a><br>
                                <br>
                                Thanks,<br>
                                <font color="#888888"> Bengt<br>
                                </font></blockquote>
                            </div>
                            <br>
                          </blockquote>
                          <br>
                        </div>
                      </div>
                    </blockquote>
                  </div>
                  <br>
                </blockquote>
                <br>
              </div>
            </div>
          </div>
        </blockquote>
      </div>
      <br>
    </blockquote>
    <br>
  </body>
</html>