Request for review 8000725: NPG: method_holder() and pool_holder() and _pool_holder field should be InstanceKlass

[David Holmes]

Hi Harold,

On 27/10/2012 4:11 AM, harold seigel wrote:
> Please review the following changes.
>
> Summary: Change the function return type of method_holder(),
> field_holder(), and pool_holder() to InstanceKlass. Also, change the
> type of field _pool_holder to InstanceKlass. These changes enabled the
> removal of lots of InstanceKlass::cast() calls from the code.

There have also been a lot of removals of Klass::cast even though the 
previous return types were Klass*. So looking at Klass::cast I see

   // Casting
   static Klass* cast(Klass* k) {
     assert(k->is_klass(), "cast to Klass");
     return k;
   }

which begs the question: how can k->is_klass() not be true? And have we 
lost anything by getting rid of these "casts"? There are additional 
checks in instanceKlass::cast too

  // Casting from Klass*
   static InstanceKlass* cast(Klass* k) {
     assert(k->is_klass(), "must be");
     assert(k->oop_is_instance(), "cast to InstanceKlass");
     return (InstanceKlass*) k;
   }

Is the need for the asserts obsolete in a NPG world?

That aside in vframe.cpp:

+  InstanceKlass*     k = m->method_holder();
    tty->print_cr("frame( sp=" INTPTR_FORMAT ", unextended_sp=" 
INTPTR_FORMAT ", fp=" INTPTR_FORMAT ", pc=" INTPTR_FORMAT ")",
                  _fr.sp(),  _fr.unextended_sp(), _fr.fp(), _fr.pc());
    tty->print("%s.%s", Klass::cast(k)->internal_name(), 
m->name()->as_C_string());

there is a Klass::cast that can be removed.

Thanks,
David
-----

>
> This webrev has a lot of files but most of the changes are small.
>
> Open webrev at http://cr.openjdk.java.net/~coleenp/bug_8000725
> <http://cr.openjdk.java.net/%7Ecoleenp/bug_8000725>
>
> Bug link at http://bugs.sun.com/view_bug.do?bug_id=8000725
>
> The changes were tested with JPRT, JCK, and other tests including ones
> for sa-jdi.
>
> Thanks, Harold