(original) (raw)

On 2/4/13 10:16 AM, Coleen Phillimore wrote:

On 2/4/2013 11:50 AM, Daniel D. Daugherty wrote:

Thanks for the additional comment. Reply below.

On 2/4/13 8:34 AM, Coleen Phillimore wrote:

On 2/4/2013 10:15 AM, Daniel D. Daugherty wrote:

Adding back the missing aliases and people...

Sorry, I meant to send this re-all.

I missed something major in my earlier review.

The metadata->is\_valid() flag should only be enabled in debug mode.  It adds a word to all metadata.

Yes, I'm aware that it adds a word to all meta data. My change to
src/share/vm/oops/metadata.hpp was intentional. Personally, I would
prefer that is\_valid() remain in the product bits for at least one
release cycle (JDK8). I'm a fan of sanity checks in product bits so
I actually wouldn't mind if it stay there permanently.

However, if you insist, I'll backout the change to
src/share/vm/oops/metadata.hpp and redo the RedefineClasses()
sanity check code to be appropriately #ifdef'ed.

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).

I enabled the setting of the non-zero bit pattern also (iirc).
However, I will back out those changes. I don't want to cause
grief for the metadata reduction effort.

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().

Yes, Karen mentioned one of those in her review and asked for
that cleanup to be tracked in a followup bug. I noticed quite
a few "type cleanups" in that area in HSX-24 and I love them.
I'm presuming that you did that work, but I haven't chased the
history to ground.

Dan

Thanks,
Coleen