<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    On 2/4/13 10:16 AM, Coleen Phillimore wrote:<br>
    <blockquote cite="mid:510FECE5.8040300@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <div class="moz-cite-prefix">On 2/4/2013 11:50 AM, Daniel D.
        Daugherty wrote:<br>
      </div>
      <blockquote cite="mid:510FE6DD.1010704@oracle.com" type="cite">
        <meta content="text/html; charset=ISO-8859-1"
          http-equiv="Content-Type">
        <tt>Thanks for the additional comment. Reply below.<br>
          <br>
          <br>
        </tt>
        <div class="moz-cite-prefix">On 2/4/13 8:34 AM, Coleen
          Phillimore wrote:<br>
        </div>
        <blockquote cite="mid:510FD51E.2040100@oracle.com" type="cite">
          <meta content="text/html; charset=ISO-8859-1"
            http-equiv="Content-Type">
          <div class="moz-cite-prefix">On 2/4/2013 10:15 AM, Daniel D.
            Daugherty wrote:<br>
          </div>
          <blockquote cite="mid:510FD08E.2050403@oracle.com" type="cite">
            <meta content="text/html; charset=ISO-8859-1"
              http-equiv="Content-Type">
            <tt>Adding back the missing aliases and people...<br>
            </tt></blockquote>
          <br>
          Sorry, I meant to send this re-all.<br>
          <br>
          I missed something major in my earlier review.<br>
          <br>
          The metadata-&gt;is_valid() flag should only be enabled in
          debug mode.&nbsp; It adds a word to all metadata.<br>
        </blockquote>
        <br>
        <tt>Yes, I'm aware that it adds a word to all meta data. My
          change to<br>
          src/share/vm/oops/metadata.hpp was intentional. Personally, I
          would<br>
          prefer that is_valid() remain in the product bits for at least
          one<br>
          release cycle (JDK8). I'm a fan of sanity checks in product
          bits so<br>
          I actually wouldn't mind if it stay there permanently.<br>
          <br>
          However, if you insist, I'll backout the change to<br>
          src/share/vm/oops/metadata.hpp and redo the RedefineClasses()<br>
          sanity check code to be appropriately #ifdef'ed.<br>
        </tt></blockquote>
      <br>
      <tt>Thanks, Dan.&nbsp; I do.&nbsp; People are actively trying to reduce
        metadata right now, so having this additional word will cause a
        regression that will be noticed for a special case.&nbsp; is_valid()
        is a bit of a hack too.&nbsp; It relies on metadata being returned
        and given a non-zero bit pattern which is only enabled in debug
        (iirc).<br>
      </tt></blockquote>
    <br>
    <tt>I enabled the setting of the non-zero bit pattern also (iirc).<br>
      However, I will back out those changes. I don't want to cause<br>
      grief for the metadata reduction effort.<br>
      <br>
      <br>
    </tt>
    <blockquote cite="mid:510FECE5.8040300@oracle.com" type="cite"><tt>
        There's also a f1_as_method and f2_as_vfinal_method calls that
        return Method* which might make the logic of your if statements
        simpler in check_no_old_or_obsolete_entries().<br>
      </tt></blockquote>
    <br>
    <tt>Yes, Karen mentioned one of those in her review and asked for<br>
      that cleanup to be tracked in a followup bug. I noticed quite<br>
      a few "type cleanups" in that area in HSX-24 and I love them.<br>
      I'm presuming that you did that work, but I haven't chased the<br>
      history to ground.<br>
      <br>
      Dan<br>
      <br>
      <br>
    </tt>
    <blockquote cite="mid:510FECE5.8040300@oracle.com" type="cite"><tt>
        <br>
        Thanks,<br>
        Coleen</tt><br>
    </blockquote>
  </body>
</html>