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

[Leela Mohan]

Hi Coleen,

I will be happy to contribute. I am planning to make the change in the
lines of Mikael was mentioning. Let me make a patch and send it to the
group soon.

Thanks,
Leela

On Fri, Jul 29, 2016 at 5:11 AM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:

>
> Leela,
>
> Do you want to contribute the fix, since you found the problem?  I will
> sponsor it for you.
>
> Thanks,
> Coleen
>
>
> On 7/29/16 4:45 AM, Mikael Gerdin wrote:
>
>> Leela,
>> Thanks for the report!
>> I filed https://bugs.openjdk.java.net/browse/JDK-8162766 to track the
>> issue.
>>
>> I must ask, how did you find this in that old 2014 webrev?
>>
>> /Mikael
>>
>> On 2016-07-29 10:35, Mikael Gerdin wrote:
>>
>>> 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
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>