<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hi Igor,<br>
      <br>
      How about changing set_method_counters() instead of adding a new
      function?<br>
      <br>
      <pre> bool set_method_counters(MethodCounters* counters) {
    if (counters == NULL) {
   †  // The store into method must be released. On platforms without
      // total store order (TSO) the reference may become visible before
      // the initialization of data otherwise.
      OrderAccess::release_store_ptr((volatile void *)&_method_counters, NULL);
      return true;
    } else {
<span class="new">      bool res = Atomic::cmpxchg_ptr(counters, (volatile void*)&_method_counters, NULL) == NULL;</span>
      if (res) {
        OrderAccess::release(); // is release need after cmpxchg?
      }
      return res;
   †}
†}

I'm not very sure if we need the release after <span class="new">cmpxchg</span>. Please check with David Holmes.
</pre>
      Thanks,<br>
      Jiangli<br>
      <br>
      On 09/17/2014 04:09 PM, Igor Veresov wrote:<br>
    </div>
    <blockquote
      cite="mid:1661EBE7-B3F1-4EF3-ADDB-428817FACCCF@oracle.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252">
      <div>Ok, here the webrev that takes care of the leak:†<a
          moz-do-not-send="true"
          href="http://cr.openjdk.java.net/%7Eiveresov/8058564/webrev.02/">http://cr.openjdk.java.net/~iveresov/8058564/webrev.02/</a></div>
      <div><br>
      </div>
      <div>Thanks,</div>
      <div>igor</div>
      <div><br>
      </div>
      <br>
      <div>
        <div>On Sep 17, 2014, at 2:36 PM, Igor Veresov <<a
            moz-do-not-send="true" href="mailto:igor.veresov@oracle.com">igor.veresov@oracle.com</a>>
          wrote:</div>
        <br class="Apple-interchange-newline">
        <blockquote type="cite">
          <div style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br
              class="Apple-interchange-newline">
            On Sep 17, 2014, at 2:26 PM, Jiangli Zhou <<a
              moz-do-not-send="true"
              href="mailto:jiangli.zhou@oracle.com">jiangli.zhou@oracle.com</a>>
            wrote:</div>
          <br class="Apple-interchange-newline" style="font-family:
            Helvetica; font-size: 12px; font-style: normal;
            font-variant: normal; font-weight: normal; letter-spacing:
            normal; line-height: normal; orphans: auto; text-align:
            start; text-indent: 0px; text-transform: none; white-space:
            normal; widows: auto; word-spacing: 0px;
            -webkit-text-stroke-width: 0px;">
          <blockquote type="cite" style="font-family: Helvetica;
            font-size: 12px; font-style: normal; font-variant: normal;
            font-weight: normal; letter-spacing: normal; line-height:
            normal; orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;">
            <div style="font-size: 12px; font-style: normal;
              font-variant: normal; font-weight: normal; letter-spacing:
              normal; line-height: normal; orphans: auto; text-align:
              start; text-indent: 0px; text-transform: none;
              white-space: normal; widows: auto; word-spacing: 0px;
              -webkit-text-stroke-width: 0px;">Hi Igor,<br>
              <br>
              On 09/17/2014 10:53 AM, Igor Veresov wrote:<br>
              <blockquote type="cite">On Sep 17, 2014, at 8:32 AM, David
                Chase <<a moz-do-not-send="true"
                  href="mailto:david.r.chase@oracle.com">david.r.chase@oracle.com</a>>
                wrote:<br>
                <br>
                <blockquote type="cite">On 2014-09-17, at 6:01 AM, Igor
                  Veresov <<a moz-do-not-send="true"
                    href="mailto:igor.veresov@oracle.com">igor.veresov@oracle.com</a>>
                  wrote:<br>
                  <br>
                  <blockquote type="cite">Alright, how about a shorter
                    fix:<span class="Apple-converted-space">†</span><a
                      moz-do-not-send="true"
                      href="http://cr.openjdk.java.net/%7Eiveresov/8058564/webrev.01/">http://cr.openjdk.java.net/~iveresov/8058564/webrev.01/</a><br>
                    <br>
                    igor<br>
                  </blockquote>
                  Does that need to be protected by a lock?<br>
                  Other than that, it looked good to me (i.e., I plugged
                  your patch into netbeans and browsed around<br>
                  and it looked like it would do what you say it does ó
                  but it also looked like it might need a lock).<br>
                  <br>
                </blockquote>
                Hm, itís a good question. There is certainly a
                semi-benign race when creating method counters. It might
                be on purpose, since having a lock there may have a
                pretty big impact on the interpreter during startup. On
                the other hand, metaspace allocation has a lock. May be
                having two locks is too much?<br>
                <br>
                Jiangli, if you remember, why do method counters have a
                racy allocation?<br>
              </blockquote>
              <br>
              I measured the possibility of the memory leak, it was rare
              in my experiments. So I ended up not using a lock to
              protect the allocation.<br>
              <br>
              Vladimir Ivanov was looking at fixing the possible memory
              leak a few month back. He was proposing using a CAS based
              solution to update the method counters. Using a lock here
              might potentially cause a deadlock.<br>
              <br>
            </div>
          </blockquote>
          <div style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>
          </div>
          <div style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;">Yup, CAS
            seems reasonable. Iíll make the change.</div>
          <div style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>
          </div>
          <div style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;">igor</div>
          <br style="font-family: Helvetica; font-size: 12px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height: normal;
            orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;">
          <blockquote type="cite" style="font-family: Helvetica;
            font-size: 12px; font-style: normal; font-variant: normal;
            font-weight: normal; letter-spacing: normal; line-height:
            normal; orphans: auto; text-align: start; text-indent: 0px;
            text-transform: none; white-space: normal; widows: auto;
            word-spacing: 0px; -webkit-text-stroke-width: 0px;">
            <div style="font-size: 12px; font-style: normal;
              font-variant: normal; font-weight: normal; letter-spacing:
              normal; line-height: normal; orphans: auto; text-align:
              start; text-indent: 0px; text-transform: none;
              white-space: normal; widows: auto; word-spacing: 0px;
              -webkit-text-stroke-width: 0px;">Thanks,<br>
              Jiangli<br>
              <br>
              <blockquote type="cite"><br>
                igor<br>
                <br>
                <br>
                <blockquote type="cite">David<br>
                  <br>
                  <blockquote type="cite">On Sep 17, 2014, at 2:35 AM,
                    Igor Veresov <<a moz-do-not-send="true"
                      href="mailto:igor.veresov@oracle.com">igor.veresov@oracle.com</a>>
                    wrote:<br>
                    <br>
                    <blockquote type="cite">Sorry, my fix is not
                      entirely good. MethodCounters should exist before
                      a method ends up in compile queue. Iíll get back
                      with the updated webrev.<br>
                      <br>
                      igor<br>
                      <br>
                      On Sep 17, 2014, at 2:01 AM, Igor Veresov <<a
                        moz-do-not-send="true"
                        href="mailto:igor.veresov@oracle.com">igor.veresov@oracle.com</a>>
                      wrote:<br>
                      <br>
                      <blockquote type="cite">We donít always have MDOs.
                        Level 1 & 2 are good examples. C2 also
                        doesnít always require an MDO.<br>
                        I also wanted it to work with other compilers,
                        like Graal. By putting this logic in the policy
                        itís in one place and I donít need to touch
                        compilers. I couldíve put it in the broker, but
                        it seemed that these level values are artifacts
                        of the policy so it seems reasonable to put it
                        in the policy.<br>
                        <br>
                        igor<br>
                        <br>
                        On Sep 17, 2014, at 12:52 AM, Vladimir Kozlov
                        <<a moz-do-not-send="true"
                          href="mailto:vladimir.kozlov@oracle.com">vladimir.kozlov@oracle.com</a>>
                        wrote:<br>
                        <br>
                        <blockquote type="cite">Why not create
                          MethodCounters in
                          Method::build_interpreter_method_data()? It is
                          called at the beginning of compilation (C1 and
                          C2) from ciMethod::ensure_method_data(). And
                          not necessary that way. My point is - why not
                          crate them at the beginning of a compilation
                          as we do with MDO? Compiled code may need to
                          access it. May be not now but in a future.<br>
                          <br>
                          Thanks,<br>
                          Vladimir<br>
                          <br>
                          On 9/17/14 12:39 AM, Igor Veresov wrote:<br>
                          <blockquote type="cite">The problem here is
                            that with -Xcomp we immediately compile a
                            method at level 3, and weíre not creating
                            MethodCounters since we never execute in the
                            interpreter and hence not setting the
                            ďhighestĒ level values. The solution is to
                            allocate MethodCounters for every method
                            compiled (unless it has been allocated
                            naturally by the interpreter). I made it in
                            a form of a callback to the policy, since
                            only tiered policies cares about these
                            values.<br>
                            <br>
                            JBS:<span class="Apple-converted-space">†</span><a
                              moz-do-not-send="true"
                              href="https://bugs.openjdk.java.net/browse/JDK-8058564">https://bugs.openjdk.java.net/browse/JDK-8058564</a><br>
                            Webrev:<span class="Apple-converted-space">†</span><a
                              moz-do-not-send="true"
                              href="http://cr.openjdk.java.net/%7Eiveresov/8058564/webrev.00">http://cr.openjdk.java.net/~iveresov/8058564/webrev.00</a><br>
                            <br>
                            Thanks,<br>
                            igor</blockquote>
                        </blockquote>
                      </blockquote>
                    </blockquote>
                  </blockquote>
                </blockquote>
              </blockquote>
            </div>
          </blockquote>
        </blockquote>
      </div>
      <br>
    </blockquote>
    <br>
  </body>
</html>