RFR 6642881: Improve performance of Class.getClassLoader()

[Coleen Phillimore]

On 6/18/14, 3:27 PM, Mandy Chung wrote:
>
> On 6/18/2014 11:35 AM, Coleen Phillimore wrote:
>>
>>     /*
>>      * Private constructor. Only the Java Virtual Machine creates Class
>>      * objects.  This constructor is not used but prevents a warning 
>> that
>>      * classLoader is not initialized.  The initialization value of 
>> non-null
>>      * prevents future JIT optimizations from assuming this final 
>> field is null.
>>      */
>>
>
> As David points out, the private constructor is to avoid the default 
> constructor being generated.  Your change to initialize the final 
> classloader field is a second reason.  The second statement reads to 
> me like the reason for this private constructor is for the warning 
> reason.  Perhaps "but prevents... " gets me confused and maybe that 
> part is not really needed?  Your change looks good anyway.

How about this?  I separated out the two concerns.

     /*
      * Private constructor. Only the Java Virtual Machine creates Class 
objects.
      * This constructor is not used and prevents the default 
constructor being
      * generated.
      */
     private Class(ClassLoader loader) {
         // Initialize final field for classLoader.  The initialization 
value of non-null
         // prevents future JIT optimizations from assuming this final 
field is null.
         classLoader = loader;
     }

Thanks!
Coleen
>
>>>
>>> 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 new field classLoader is explicit and not injected by the jvm. 
>> The jvm looks up the offset of the field when it lays out the class 
>> Class and initializes it directly in the instance of 
>> java.lang.Class.  So it isn't added to the injected fields.
>
> Right... I missed that obvious one.
>
>>>
>>> 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?
>>
>> I didn't completely follow the relationship between Chris's patch and 
>> this bug.   Yes, I'll file a new bug or ask someone who knows more 
>> about it to do so.
>>
>
> Thanks.  The patch requires more investigation in the bootstrapping 
> side of thing.
>
>> The plan is to push both through hs-rt.
>
> Great.  That's what I wanted to know.
>
> Mandy
>
>