<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    Hi John,<br>
    <br>
    On 2011-11-18 18:11, John Cuthbertson wrote:
    <blockquote cite="mid:4EC691AD.20901@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <title></title>
      Hi Bengt,<br>
      <br>
      The .03 version looks good to me. One minor nit - can you place
      parentheses around the MarkSweepAlwaysCompactCount < 1
      expressions?
      O know they're not required but, to my eye at least, thy help
      separating the expressions used in the or.<br>
    </blockquote>
    <br>
    Good point. I'll add the parentheses.<br>
    <br>
    All set to push this now. Thanks Ramki, Jon, John and Tony for the
    reviews!<br>
    <br>
    Bengt<br>
    <br>
    <blockquote cite="mid:4EC691AD.20901@oracle.com" type="cite">
      <br>
      Thanks,<br>
      <br>
      JohnC<br>
      <br>
      On 11/17/11 23:53, Bengt Rutisson wrote:
      <blockquote cite="mid:4EC60EDD.2010106@oracle.com" type="cite">
        <meta content="text/html; charset=ISO-8859-1"
          http-equiv="Content-Type">
        <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 moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.03/">http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/</a><br>
        <br>
        Instead of this (forbid values < 1):<br>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Ebrutisso/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="border-left: 1px
              solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex;
              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="border-left: 1px solid rgb(204, 204,
                          204); margin: 0pt 0pt 0pt 0.8ex; 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="border-left: 1px solid
                                    rgb(204, 204, 204); margin: 0pt 0pt
                                    0pt 0.8ex; 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="border-left: 1px solid
                                    rgb(204, 204, 204); margin: 0pt 0pt
                                    0pt 0.8ex; 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>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>