<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix"><br>
      Hi Dean,<br>
      <br>
      Thanks for looking at this!<br>
      <br>
      On 1/19/13 6:51 AM, Dean Long wrote:<br>
    </div>
    <blockquote cite="mid:50FA347C.8020704@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      How about checking that the returned "len" is as expected:<br>
      <br>
      <tt> &nbsp;&nbsp; if (sysctl(mib, 2, &amp;mem_val, &amp;len, NULL, 0) != -1)
        {<br>
        &nbsp; &nbsp;&nbsp; assert(len == sizeof(mem_val), "oops");<br>
        &nbsp;&nbsp;&nbsp;&nbsp; _physical_memory = mem_val;</tt><br>
      <br>
      which would have detected the problem with HW_USERMEM.<br>
    </blockquote>
    <br>
    Good idea. I added the assert. I also added it to the only other
    call to sysctl in the hotspot source. That call is in the same
    method.<br>
    <br>
    I also tried reverting to HW_USERMEM and the assert catches the
    issue with that key. We get 4 instead of 8 in len.<br>
    <br>
    Updated webrev:<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/8006431/webrev.02/">http://cr.openjdk.java.net/~brutisso/8006431/webrev.02/</a><br>
    <br>
    Thanks,<br>
    Bengt<br>
    <br>
    <blockquote cite="mid:50FA347C.8020704@oracle.com" type="cite"> <br>
      dl<br>
      <br>
      On 1/18/2013 6:31 AM, Bengt Rutisson wrote:
      <blockquote cite="mid:50F95CAC.1090107@oracle.com" type="cite">
        <meta content="text/html; charset=ISO-8859-1"
          http-equiv="Content-Type">
        <div class="moz-cite-prefix"><br>
          Hi Vitaly,<br>
          <br>
          Thanks for looking at this!<br>
          <br>
          I updated the comment. Here is an updated webrev:<br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Ebrutisso/8006431/webrev.01/">http://cr.openjdk.java.net/~brutisso/8006431/webrev.01/</a><br>
          <br>
          On 1/18/13 3:11 PM, Vitaly Davidovich wrote:<br>
        </div>
        <blockquote
cite="mid:CAHjP37HoKAjJHDGgpTQU-Wm=J-UMcXqnC=Eap6KEVvq4gVTUxQ@mail.gmail.com"
          type="cite">
          <p dir="ltr">Also, is it worthwhile to initialize mem_value to
            0 explicitly before passing it to the syscall? That should
            avoid garbage in the upper 32 bits; I realize it's not
            needed now but maybe for completeness sake?</p>
        </blockquote>
        <br>
        HW_MEMSIZE is explictly documented to be a 64 bit value. I don't
        think it should be necessary to initialize mem_value to 0 now.<br>
        <br>
        Thanks again for the review!<br>
        Bengt<br>
        <br>
        <br>
        <blockquote
cite="mid:CAHjP37HoKAjJHDGgpTQU-Wm=J-UMcXqnC=Eap6KEVvq4gVTUxQ@mail.gmail.com"
          type="cite">
          <p dir="ltr">Sent from my phone</p>
          <div class="gmail_quote">On Jan 18, 2013 8:57 AM, "Vitaly
            Davidovich" &lt;<a moz-do-not-send="true"
              href="mailto:vitalyd@gmail.com">vitalyd@gmail.com</a>&gt;
            wrote:<br type="attribution">
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <p dir="ltr">Hi Bengt,</p>
              <p dir="ltr">The comment there needs to be updated because
                it's still talking about usermem.</p>
              <p dir="ltr">Thanks</p>
              <p dir="ltr">Sent from my phone</p>
              <div class="gmail_quote">On Jan 18, 2013 7:48 AM, "Bengt
                Rutisson" &lt;<a moz-do-not-send="true"
                  href="mailto:bengt.rutisson@oracle.com"
                  target="_blank">bengt.rutisson@oracle.com</a>&gt;
                wrote:<br type="attribution">
                <blockquote class="gmail_quote" style="margin:0 0 0
                  .8ex;border-left:1px #ccc solid;padding-left:1ex">
                  <div bgcolor="#FFFFFF" text="#000000">
                    <div><br>
                      <br>
                      Hi Mikael,<br>
                      <br>
                      Thanks for the review!<br>
                      <br>
                      On 1/18/13 8:09 AM, Mikael Vidstedt wrote:<br>
                    </div>
                    <blockquote type="cite">
                      <div><br>
                      </div>
                      <div>L<span>ooks good, assuming u_long is a 64-bit
                          type.</span></div>
                    </blockquote>
                    <br>
                    Good point. It seems like u_long is a 64 bit value
                    on OSX, but that's not guaranteed since it is just
                    "unsigned long". I changed mem_val to be julong,
                    which should always be 64 bit. (The instance
                    variable _physical_memory is also a julong.)<br>
                    <br>
                    So, now it is a two-word review request&nbsp; :)<br>
                    <br>
                    diff --git a/src/os/bsd/vm/os_bsd.cpp
                    b/src/os/bsd/vm/os_bsd.cpp<br>
                    --- a/src/os/bsd/vm/os_bsd.cpp<br>
                    +++ b/src/os/bsd/vm/os_bsd.cpp<br>
                    @@ -243,7 +243,7 @@<br>
                    &nbsp;&nbsp; int mib[2];<br>
                    &nbsp;&nbsp; size_t len;<br>
                    &nbsp;&nbsp; int cpu_val;<br>
                    -&nbsp; u_long mem_val;<br>
                    +&nbsp; julong mem_val;<br>
                    &nbsp;<br>
                    &nbsp;&nbsp; /* get processors count via hw.ncpus sysctl */<br>
                    &nbsp;&nbsp; mib[0] = CTL_HW;<br>
                    @@ -260,7 +260,7 @@<br>
                    &nbsp;&nbsp;&nbsp; * instead of hw.physmem because we need size of
                    allocatable memory<br>
                    &nbsp;&nbsp;&nbsp; */<br>
                    &nbsp;&nbsp; mib[0] = CTL_HW;<br>
                    -&nbsp; mib[1] = HW_USERMEM;<br>
                    +&nbsp; mib[1] = HW_MEMSIZE;<br>
                    &nbsp;&nbsp; len = sizeof(mem_val);<br>
                    &nbsp;&nbsp; if (sysctl(mib, 2, &amp;mem_val, &amp;len, NULL,
                    0) != -1)<br>
                    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; _physical_memory = mem_val;<br>
                    <br>
                    <br>
                    Thanks,<br>
                    Bengt<br>
                    &nbsp;<br>
                    <br>
                    <blockquote type="cite">
                      <div><br>
                        /Mikael</div>
                      <div><br>
                        On 17 jan 2013, at 22:36, Bengt Rutisson &lt;<a
                          moz-do-not-send="true"
                          href="mailto:bengt.rutisson@oracle.com"
                          target="_blank">bengt.rutisson@oracle.com</a>&gt;


                        wrote:<br>
                        <br>
                      </div>
                      <blockquote type="cite">
                        <div> <br>
                          Hi all,<br>
                          <br>
                          Could I have a couple of reviews for this
                          small fix?<br>
                          <br>
                          <a moz-do-not-send="true"
                            href="http://cr.openjdk.java.net/%7Ebrutisso/8006431/webrev.00/"
                            target="_blank">http://cr.openjdk.java.net/~brutisso/8006431/webrev.00/</a><br>
                          <br>
                          On OSX we used HW_USERMEM value from sysctl()
                          to get the available physical memory on the
                          machine. This is a 32 bit value but we store
                          it in a 64 bit variable. This means that we
                          get kind of random and normally very large
                          values for what we think the physical memory
                          is.<br>
                          <br>
                          We actually don't want a 32 bit value since we
                          want to handle machines with more than 2 or 4
                          GB of memory. Instead of HW_USERMEM we should
                          be using HW_MEMSIZE which will give us a 64
                          bit value.<br>
                          <br>
                          See:<br>
                          <a moz-do-not-send="true"
href="https://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man3/sysctl.3.html"
                            target="_blank">https://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man3/sysctl.3.html</a><br>
                          <br>
                          Thanks Staffan Larsen for both detecting the
                          problem and providing a solution.<br>
                          <br>
                          This is a one-word-change. So, to save you a
                          mouse click on the webrev link above, here is
                          the diff:<br>
                          <br>
                          --- a/src/os/bsd/vm/os_bsd.cpp<br>
                          +++ b/src/os/bsd/vm/os_bsd.cpp<br>
                          @@ -260,7 +260,7 @@<br>
                          &nbsp;&nbsp;&nbsp; * instead of hw.physmem because we need
                          size of allocatable memory<br>
                          &nbsp;&nbsp;&nbsp; */<br>
                          &nbsp;&nbsp; mib[0] = CTL_HW;<br>
                          -&nbsp; mib[1] = HW_USERMEM;<br>
                          +&nbsp; mib[1] = HW_MEMSIZE;<br>
                          &nbsp;&nbsp; len = sizeof(mem_val);<br>
                          &nbsp;&nbsp; if (sysctl(mib, 2, &amp;mem_val, &amp;len,
                          NULL, 0) != -1)<br>
                          &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; _physical_memory = mem_val;<br>
                          <br>
                          Thanks,<br>
                          Bengt<br>
                        </div>
                      </blockquote>
                    </blockquote>
                    <br>
                  </div>
                </blockquote>
              </div>
            </blockquote>
          </div>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>