<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->is_valid() flag should only be enabled in
debug mode. 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. I do. 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. is_valid()
is a bit of a hack too. 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>