[8u40] RFR 6642881: Improve performance of Class.getClassLoader()

[Mikael Gerdin]

Hi,

(dropping core-libs since this is a VM-specific issue)

On 2016-07-29 09:55, Leela Mohan wrote:
> Hi David,
>
> I understand, Klass types are no longer oops  but
> JNIHandles::resolve_non_null()
> would expose naked oops. In other words, KlassOops are no longer oops but
> java.lang.Class objects are.

I agree.
In this case it's not only the java.lang.Class but also the ClassLoader oop.
In an unrelated change I've toyed with adding an assertion that the 
current thread is not _thread_in_native in JNIHandles::resolve.. to 
catch similar problems.

The code in the webrev explicitly transitions to native and as such 
there can be a GC in progress when get_class_loader is being executed 
and while resolving the handles and creating new ones are probably not 
going to crash we could be creating an invalid handle at line 962.

I'm unable to locate the corresponding code in JDK 9 but in 8u the 
potential problem persists.

The 8u UnsafeDefineClass0 needs the native transition because the JVM_ 
entrypoints expect it but all of them just transition back to _in_vm so 
if the code could just stay in the VM and use internal VM APIs to get 
the caller class and protection domains this would of course not be an 
issue.


/Mikael

>
> Thanks,
> Leela
>
> On Thu, Jul 28, 2016 at 10:51 PM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Leela,
>>
>> On 29/07/2016 12:59 PM, Leela Mohan wrote:
>>
>>> I think, change in the file unsafe.cpp is incorrect. (
>>> http://cr.openjdk.java.net/~coleenp/6642881_8u40_hotspot/ )
>>>
>>> Below function is accessing naked oops when thread has transitioned to
>>> "native":
>>>
>>> *+ static jobject get_class_loader(JNIEnv* env, jclass cls) {**+   if
>>> (java_lang_Class::is_primitive(JNIHandles::resolve_non_null(cls)))
>>> {**+     return NULL;**+   }**+   Klass* k =
>>> java_lang_Class::as_Klass(JNIHandles::resolve_non_null(cls));**+   oop
>>> loader = k->class_loader();**+   return JNIHandles::make_local(env,
>>> loader);**+ }*
>>>
>>
>> klass types are no longer oops in the Java heap, but metaspace objects
>> that reside in a per-classloader allocation region. They never get
>> compacted or relocated so raw pointers to them are safe.
>>
>> Cheers,
>> David
>>
>>
>>
>>> - Leela
>>>
>>> On Mon, Sep 8, 2014 at 7:05 PM, Coleen Phillimore <
>>> coleen.phillimore at oracle.com> wrote:
>>>
>>>
>>>> Thanks, Mandy!
>>>>
>>>> Coleen
>>>>
>>>>
>>>> On 9/8/14, 6:59 PM, Mandy Chung wrote:
>>>>
>>>> Thumbs up.
>>>>>
>>>>> Mandy
>>>>>
>>>>> On 9/5/2014 12:55 PM, Coleen Phillimore wrote:
>>>>>
>>>>> Summary: Add classLoader to java/lang/Class instance for fast access
>>>>>>
>>>>>> This is a backport request for 8u40.   This change has been in the jdk9
>>>>>> code for 3 months without any problems.
>>>>>>
>>>>>> The JDK changes hg imported cleanly.  The Hotspot change needed a hand
>>>>>> merge for create_mirror call in klass.cpp.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~coleenp/6642881_8u40_jdk/
>>>>>> http://cr.openjdk.java.net/~coleenp/6642881_8u40_hotspot/
>>>>>>
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-6642881
>>>>>>
>>>>>> Ran jdk_core jtreg tests in jdk with both jdk/hotspot changes. Also ran
>>>>>> jck java_lang tests with only the hotspot change.  The hotspot change
>>>>>> can
>>>>>> be tested separately from the jdk change (but not the other way
>>>>>> around).
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>