<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <font size="+1">Dan, Thank you for reviewing the code.&nbsp;&nbsp; Most of the
      files were easy.<br>
      <br>
    </font>
    <div class="moz-cite-prefix">On 2/14/14 10:15 AM, Daniel D.
      Daugherty wrote:<br>
    </div>
    <blockquote cite="mid:52FE3306.50900@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      On 2/13/14 12:00 PM, Coleen Phillimore wrote:<br>
      <blockquote cite="mid:52FD1653.2030809@oracle.com" type="cite">
        <meta http-equiv="content-type" content="text/html;
          charset=ISO-8859-1">
        <font size="+1">Summary: Remove search in system dictionary and
          hacks, replace with verifying in CLD::_klasses list.<br>
          <br>
          open webrev at <a moz-do-not-send="true"
            class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Ecoleenp/8027146/">http://cr.openjdk.java.net/~coleenp/8027146/</a><br>
        </font></blockquote>
      <tt><font size="+1">&nbsp;<br>
          <font size="+1">src/share/vm/classfile/classLoaderData.<font
              size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No<font size="+1"> comments<font
                  size="+1"> other than why add a blank line on 273<font
                    size="+1">?<br>
                  </font></font></font></font></font></font></tt></blockquote>
    <br>
    <tt><font size="+1">No reason.&nbsp; I'll remove it.&nbsp; I might have made
        and removed a different change there.<br>
        <br>
      </font></tt>
    <blockquote cite="mid:52FE3306.50900@oracle.com" type="cite"><tt><font
          size="+1"><font size="+1"><font size="+1"><font size="+1"><font
                  size="+1"><font size="+1"> <br>
                  </font></font></font></font>src/share/vm/classfile/classLoaderData.<font
              size="+1">c</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/classfile/dictionary.cpp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/classfile/systemDictionary.<font
              size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No com<font size="+1">ments.<br>
                <br>
              </font></font>src/share/vm/classfile/systemDictionary.<font
              size="+1">c</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; So
              SystemDictionary::verify_obj_klass_present() isn't even<br>
              <font size="+1">&nbsp;&nbsp;&nbsp; useful or correct in sanity
                checking/debug code?<br>
              </font></font></font></font></tt></blockquote>
    <tt><font size="+1"><br>
        I don't think it's that useful since there are so many
        exceptions to this check and it has to lock the system dictionary
        for the query.&nbsp;&nbsp; And it used to be called when verifying the
        classes in the system dictionary.<br>
        <br>
      </font></tt>
    <blockquote cite="mid:52FE3306.50900@oracle.com" type="cite"><tt><font
          size="+1"><font size="+1"><font size="+1"><font size="+1"> <br>
                <font size="+1">&nbsp;&nbsp;&nbsp; Update: So
                  InstanceKlass::verify_on() now does a much simpler<br>
                  <font size="+1">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; <font size="+1">contains_klass()

                      check. I <font size="+1">understand now why the
                        existing<br>
                        <font size="+1">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; check needs to be
                          replaced by the simpler and less strict<br>
                          <font size="+1">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; check.</font><br>
                        </font></font></font></font></font></font></font></font></font></tt></blockquote>
    <br>
    <tt><font size="+1">Yes, when we create the class we link it to it's
        ClassLoaderData and vice versa.&nbsp; Making sure these links are
        correct is worthwhile.<br>
        <br>
      </font></tt>
    <blockquote cite="mid:52FE3306.50900@oracle.com" type="cite"><tt><font
          size="+1"><font size="+1"><font size="+1"><font size="+1"><font
                  size="+1"><font size="+1"><font size="+1"><font
                        size="+1"><font size="+1"> </font></font></font></font></font><br>
              </font></font>src/share/vm/oops/arrayKlass.<font size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/arrayKlass.<font size="+1">c</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/instanceKlass.<font size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/instanceKlass.<font size="+1">c</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; The old c<font size="+1">ode didn't call
                verify_obj_klass_present() <font size="+1">unless<br>
                  <font size="+1">&nbsp;&nbsp;&nbsp; is_loaded() is tr<font size="+1">ue.

                      The new code presumes that <font size="+1">is_loaded()<br>
                        <font size="+1">&nbsp;&nbsp;&nbsp; is true <font size="+1">since

                            it expects the class to be present in the<br>
                            <font size="+1">&nbsp;&nbsp;&nbsp; class_loader_data.<br>
                              <br>
                              <font size="+1">&nbsp;&nbsp;&nbsp; Update: <font
                                  size="+1"><font size="+1">To put it
                                    another way<font size="+1">: The old
                                      code assume<font size="+1">d when<br>
                                        <font size="+1">&nbsp;&nbsp;&nbsp; is_loaded()
                                          is true that the class was in
                                          the system dictionary<br>
                                          <font size="+1">&nbsp;&nbsp;&nbsp; or in the
                                            placeholder table. That's
                                            not quite correct in all<br>
                                            <font size="+1">&nbsp;&nbsp;&nbsp; cases.
                                              When is_loaded() i<font
                                                size="+1">s true,<font
                                                  size="+1"> all we <font
                                                    size="+1">know for
                                                    sure <font
                                                      size="+1">is that<br>
                                                      <font size="+1">&nbsp;&nbsp;&nbsp;

                                                        the class must
                                                        be present in<font
                                                          size="+1"> the
                                                          class_loader_data();

                                                          it might<br>
                                                          <font
                                                          size="+1">&nbsp;&nbsp;&nbsp;
                                                          still be "on
                                                          its way" to
                                                          either the
                                                          system
                                                          dictionary or
                                                          the<br>
                                                          <font
                                                          size="+1">&nbsp;&nbsp;&nbsp;
                                                          placeholder
                                                          table, but has
                                                          not yet
                                                          arrived there.</font><br>
                                                          </font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></tt></blockquote>
    <br>
    <tt><font size="+1">Yes.&nbsp; </font></tt><tt><font size="+1">I think
        this is the better verification.<br>
        <br>
        Thanks!<br>
        Coleen<br>
      </font></tt>
    <blockquote cite="mid:52FE3306.50900@oracle.com" type="cite"><tt><font
          size="+1"><font size="+1"><font size="+1"><font size="+1"><font
                  size="+1"><font size="+1"><font size="+1"><font
                        size="+1"><font size="+1"><font size="+1"><font
                              size="+1"><font size="+1"><font size="+1"><font
                                    size="+1"><font size="+1"><font
                                        size="+1"><font size="+1"><font
                                            size="+1"><font size="+1"><font
                                                size="+1"><font
                                                  size="+1"><font
                                                    size="+1"><font
                                                      size="+1"><font
                                                        size="+1"><font
                                                          size="+1"><font
                                                          size="+1"> </font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font></font><br>
            </font>src/share/vm/oops/klass.<font size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/klass.<font size="+1">c</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/objArrayKlass.<font size="+1">h</font>pp<br>
            <font size="+1">&nbsp;&nbsp;&nbsp; No comments.<br>
              <br>
            </font>src/share/vm/oops/objArrayKlass.<font size="+1">c</font>pp<br>
            &nbsp;&nbsp;&nbsp; No comments.<br>
            <br>
            Thumbs up.<br>
            <br>
            <font size="+1">Dan<br>
              <br>
              <br>
            </font><font size="+1">&nbsp;</font><br>
          </font></font></tt>
      <blockquote cite="mid:52FD1653.2030809@oracle.com" type="cite"><font
          size="+1"> bug link <a moz-do-not-send="true"
            class="moz-txt-link-freetext"
            href="https://bugs.openjdk.java.net/browse/JDK-8027146">https://bugs.openjdk.java.net/browse/JDK-8027146</a><br>
          <br>
          Tested with AllocClasses.java with VM_Verify op suggested in
          the bug report (can't add it's noreg-hard).&nbsp;&nbsp; Also ran all
          jtreg and gc jtreg tests with -XX:+VerifyBeforeGC.<br>
          <br>
          Thanks,<br>
          Coleen<br>
        </font> </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>