RFR(S) JDK-8178350 - klassVtable and klassItable should be ValueObj

[coleen.phillimore at oracle.com]

I like this change.  It's always been a surprise to need a ResourceMark 
around klass->vtable() calls.

On 4/13/17 5:22 AM, Ioi Lam wrote:
> Hi, please review this JVM start-up enhancement:
>
> https://bugs.openjdk.java.net/browse/JDK-8178350
> http://cr.openjdk.java.net/~iklam/jdk10/8178350-klassvtable-as-valueobj.v01/ 
>
>
> Klass::vtable() is called frequently inside 
> InstanceKlass::link_class_impl(). We have a start-up benchmark that 
> loads about 4000 classes. Klass::vtable() is called 32458 times and 
> costs about 2~3% of a total elapsed time of 720 ms.
>

Nice when cleaning something up, you get better performance!

> Because klassVtable and klassItable are very simple, fixed sized 
> structures, it's much better to allocate them on the stack:
>
> OLD:
>
> class klassVtable : ResourceObj {...}
> klassVtable* Klass::vtable() const {
>   return new klassVtable(const_cast<Klass*>(this), start_of_vtable(), 
> vtable_length() / vtableEntry::size());
> }
>
> NEW:
>
> class klassVtable VALUE_OBJ_CLASS_SPEC {...}
> klassVtable Klass::vtable() const {
>   return klassVtable(const_cast<Klass*>(this), start_of_vtable(), 
> vtable_length() / vtableEntry::size());
> }
>
> (>> David) I am not sure whether to make klassVtable a StackObj or 
> ValueObj. Since it's returned by a function call, it seems ValueObj is 
> better??

I don't have a strong opinion about this.  ValueObj makes sense because 
StackObj would be coded as something like, instead of this:

+ Method * object_resolved_method = object_klass->vtable().method_at(index);


Would be this:

     KlassVtable vt(object_klass);
     Method* object_resolved_method = vt.method_at(index);

So because of how it's called, it makes more sense as a ValueObj (which 
we want to get rid of).  Neither can call CHEAP operator new.
>
> I also removed some ResourceMarks that are obviously not necessary. I 
> left some others unchanged, like here in link_class_impl():
>
>  623         ResourceMark rm(THREAD);
>  624         vtable().initialize_vtable(true, CHECK_false);
>  625         itable().initialize_itable(true, CHECK_false);
>
> because some code inside initialize_vtable() would do a resource 
> allocation without a ResourceMark. I don't want to touch that for now, 
> as it's unclear to me whether the allocated object(s) should be 
> scoped, or should be returned to a higher scope. I filed JDK-8178712 
> to track this.
>

Seems ok to me.
thanks,
Coleen
> Thanks
> - Ioi