RFR: 8138659: Speed up InstanceKlass subclass discrimination

[Stefan Karlsson]

Hi Kim,

On 2015-10-03 20:26, Kim Barrett wrote:
> Please review this change to speed up discrimination among the classes
> rooted at InstanceKlass.
>
> We use 2 bits from InstanceKlass::_misc_flags (the _misc_kind field)
> for discrimination among the four kinds of InstanceKlass.  We use the
> low 2 bits of _misc_flags to avoid a shift when accessing the field.
> This means that all the existing flags are being shifted up 2 bits.
> Added an accessor and a suite of predicates to InstanceKlass.  Added
> kind argument to InstanceKlass constructor, which is used to set the
> _misc_kind field. Updated all callers to provide the appropriate
> value.
>
> With these changes we can use the layout_helper-based
> Klass::oop_is_instance to get to the InstanceKlass case, and use the
> _misc_kind value to further discriminate from there.  In particular,
> the concrete InstanceKlass case is (after inlining)
>
>    (klass->_layout_helper > 0) &&
>    ((static_cast<InstanceKlass*>(klass)->_misc_flags & 3) == 0)
>
> We could do better with a single discriminator, but there doesn't
> appear to be a good place to add such in Klass, and we don't want to
> make Klass bigger.
>
> Changed calls to Klass::oop_is_instanceXXX to instead call
> Klass::oop_is_instance then cast to InstanceKlass and use the
> corresponding kind_is_xxx predicate.  Removed the no longer used
> Klass::oop_is_instanceXXX functions.
>
> Removed unused InstanceRefKlass::cast.  InstanceMirrorKlass::cast is
> retained because it is used.  InstanceClassLoaderKlass::cast does not
> exist.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8138659
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8138659/webrev.00/

Did you consider keeping the "kind" infrastructure within the 
instanceKlass files instead of letting it leak down into oop.inline.hpp 
and out to the rest of the runtime?

- virtual bool oop_is_instanceClassLoader() const { return false; }
- virtual bool oop_is_instanceMirror() const { return false; }
- virtual bool oop_is_instanceRef() const { return false; }


Could be:

bool oop_is_instanceClassLoader() const { return oop_is_instance() && 
InstanceKlass::cast(k)->oop_is_class_loader(); }
bool oop_is_instanceMirror() const           { return oop_is_instance() 
&& InstanceKlass::cast(k)->oop_is_mirror(); }
bool oop_is_instanceRef() const                { return 
oop_is_instance() && InstanceKlass::cast(k)->oop_is_reference(); }

and then the following change would be unnecessary:

- assert(SystemDictionary::Class_klass()->oop_is_instanceMirror(), "Is 
not?");
+ 
assert(InstanceKlass::cast(SystemDictionary::Class_klass())->kind_is_mirror(), 
"Is not?");


Other than that, this looks good to me.

StefanK

>
> Testing:
> JPRT
> Aurora ad-hoc defaults + GC Nightly + Runtime Nightly
>