RFR 6642881: Improve performance of Class.getClassLoader()

[Coleen Phillimore]

Hi, I missed a question.

On 6/18/14, 2:16 PM, Mandy Chung wrote:
> Hi Coleen,
>
> I'm glad to see this change to go in.  Thanks for doing this.  The jdk 
> change looks good.  Minor comment:
>
>  684     // Initialized in JVM not by fake constructor
>  685     private final ClassLoader classLoader;
>
> I prefer to say "Initialized in JVM not by the private constructor"
> and the comment of the private constructor perhaps can say:
>
>      /*
>       * Private constructor.Only the Java Virtual Machine creates Class
>       * objects.  This constructor is not used and JVM directly injects
>       * value of the instance fields.
>       */
>      private Class(ClassLoader loader) {
>          classLoader = loader;
>      }
>
> And It may be worth to mention that the classLoader field is 
> initialized to
> the given loader parameter instead of null to prepare for possible 
> future JIT optimization.
>
> I didn't review the hotspot change.  The new classLoader field is not
> in CLASS_INJECTED_FIELDS - is it just temporary to avoid the synchronized
> jdk/hotspot change?  What if you push both changes through the same 
> forest jdk9/hs-rt?

The plan is to push both through hs-rt.   The jdk changes don't work 
without the jvm changes but I think that's fine because both will hit 
jdk9/dev at the same time.  I've prewarned the gatekeepers and 
Alejandro.  The hotspot change works without the jdk change because most 
people on hotspot don't build full jdks during development (yet).

Thanks,
Coleen
>
> A side note - Christian has a patch attached in this bug that can
> improve the performance when running with SecurityManager on the
> classloader related check for permission.   Can you create a new
> bug to track that so that it won't get lost once this bug is
> resolved?
>
> thanks
> Mandy
>
> On 6/16/2014 10:30 PM, David Holmes wrote:
>> On 17/06/2014 6:43 AM, Coleen Phillimore wrote:
>>>
>>>
>>>      /*
>>>       * 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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>