RFR 6642881: Improve performance of Class.getClassLoader()

[roger riggs]

Hi,

I'd favor David's suggestion also, omitting the constructor and 
initializing to null.

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
>>>>>
>>>>>
>>>>
>>>
>>