RFR 6642881: Improve performance of Class.getClassLoader()

[David Holmes]

On 18/06/2014 1:33 AM, Christian Thalinger wrote:
>
> On Jun 17, 2014, at 7:52 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>
>>
>> On 6/17/14, 10:08 AM, roger riggs wrote:
>>> Hi,
>>>
>>> I'd favor David's suggestion also, omitting the constructor and initializing to null.

First I didn't suggest ommitting the constructor, but leaving the 
existing constructor alone. There must be a private constructor present 
to stop javac from generating one.

>> I think I can't actually do this because as a final field, won't the JIT optimize it to always be null?
>
> Coleen and I talked about this.  Currently, AFAIK, our JITs don’t do this but we might in the future (or someone else when we are all not around anymore).  I suggested this solution to be future-proof.

I can't quite envisage how the JIT would go and look at the bytecode for 
the initializer to see that it was null, but ...

David

>>
>> Coleen
>>
>>>
>>> Roger
>>>
>>> On 6/17/2014 1:30 AM, David Holmes wrote:
>>>> On 17/06/2014 6:43 AM, Coleen Phillimore wrote:
>>>>>
>>>>> On 6/16/14, 1:50 PM, roger riggs wrote:
>>>>>> Hi Colleen,
>>>>>>
>>>>>> In Class.java, if only the VM creates Class objects (comment at line
>>>>>> 681),
>>>>>> and it does not use the constructor then the code (line 136) in the
>>>>>> constructor
>>>>>> to set classloader is misleading and deserves a comment.
>>>>>
>>>>> Thanks, Roger.  How about this comment?
>>>>>
>>>>>      /*
>>>>>       * Constructor. Only the Java Virtual Machine creates Class
>>>>>       * objects, but this constructor prevents a warning that
>>>>> classLoader is not
>>>>>       * initialized.
>>>>>       */
>>>>>      private Class(ClassLoader loader) {
>>>>>          classLoader = loader;
>>>>>      }
>>>>
>>>> Why not just initialize classloader to null when it is declared and leave the constructor alone? The constructor exists to prevent javac from creating a default constructor, and thus ensuring Class instances can not be created (which is what the original comment should, but doesn't quite, say).
>>>>
>>>> David
>>>>
>>>>> Coleen
>>>>>>
>>>>>> Roger
>>>>>>
>>>>>> On 6/16/2014 1:45 PM, Lois Foltan wrote:
>>>>>>> Hi Coleen,
>>>>>>> This looks good.
>>>>>>> Lois
>>>>>>>
>>>>>>> On 6/16/2014 11:42 AM, Coleen Phillimore wrote:
>>>>>>>> Summary: Add classLoader to java/lang/Class instance for fast access
>>>>>>>>
>>>>>>>> In order to improve performance of Class.getClassLoader() in a way
>>>>>>>> to allow the compilers to automatically optimize this call, I added
>>>>>>>> the classLoader to the instance of java/lang/Class.  For
>>>>>>>> microbenchmarks, this results in a 98% improvement, which was
>>>>>>>> expected.  For Oracle internal applications, this results in a
>>>>>>>> 10-12% improvement on solaris/sparc.   The increase in size of
>>>>>>>> java/lang/Class can be offset by other changes (removing constant
>>>>>>>> pool lock, or removing signers).
>>>>>>>>
>>>>>>>> See bug link https://bugs.openjdk.java.net/browse/JDK-6642881 and
>>>>>>>> associated linked bugs for more details.
>>>>>>>>
>>>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6642881_hotspot/
>>>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk/
>>>>>>>>
>>>>>>>> There is both hotspot and jdk changes for this change. The hotspot
>>>>>>>> changes can work without the jdk changes (check for optional field),
>>>>>>>> but not vice-versa.  I'll file another bug (and compatibility
>>>>>>>> request) to remove JVM_GetClassLoader from hotspot.
>>>>>>>>
>>>>>>>> Tested with jck lang, vm, api/java_lang tests with/without jdk
>>>>>>>> change, nsk vm.quick.testlist and hotspot jtreg tests, and jprt.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>
>