RFR 6642881: Improve performance of Class.getClassLoader()

[Mandy Chung]

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?

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