RFR 8047737 Move array component mirror to instance of java/lang/Class

[Christian Thalinger]

On Jun 30, 2014, at 1:06 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:

> 
> On 6/30/14, 3:50 PM, Christian Thalinger wrote:
>>      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;
>> +        componentType = null;
>>      }
>> 
>> Are we worried about the same optimization?
> 
> I don't know if I was justified in worrying about the optimization in the first place.   Since getComponentType() is conditional, I wasn't worried.
> 
> But it should be consistent.  Maybe I should revert the classLoader constructor change, now that I fixed all the tests not to care.   What do people think?
>> 
>> +  compute_optional_offset(_component_mirror_offset,
>> +                 klass_oop, vmSymbols::componentType_name(),
>> +                 vmSymbols::class_signature());
>> 
>> Is there a followup cleanup to make it non-optional?  Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
> 
> Yes, please look at the _full webrev.  That has the non-optional changes in it and the follow on changes to remove getComponentType as an intrinsic in C2 (will file new RFE).  I really would like a compiler person to comment on it.

Sorry, I missed that.  Yes, the compiler changes look good.

> 
> Thanks so much,
> Coleen
> 
>> 
>> On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>> 
>>> 
>>> On 6/30/14, 1:55 AM, David Holmes wrote:
>>>> Hi Coleen,
>>>> 
>>>> Your webrev links are to internal locations.
>>> 
>>> Sorry, I cut/pasted the wrong links.  They are:
>>> 
>>> http://cr.openjdk.java.net/~coleenp/8047737_jdk/
>>> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
>>> 
>>> and the full version
>>> 
>>> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
>>> 
>>> Thank you for pointing this out David.
>>> 
>>> Coleen
>>> 
>>>> 
>>>> David
>>>> 
>>>> On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
>>>>> Summary: Add field in java.lang.Class for componentType to simplify oop
>>>>> processing and intrinsics in JVM
>>>>> 
>>>>> This is part of ongoing work to clean up oop pointers in the metadata
>>>>> and simplify the interface between the JDK j.l.C and the JVM. There's a
>>>>> performance benefit at the end of all of this if we can remove all oop
>>>>> pointers from metadata.   mirror in Klass is the only one left after
>>>>> this full change.
>>>>> 
>>>>> See bug https://bugs.openjdk.java.net/browse/JDK-8047737
>>>>> 
>>>>> There are a couple steps to this change because Hotspot testing is done
>>>>> with promoted JDKs.  The first step is this webrev:
>>>>> 
>>>>> http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/
>>>>> http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
>>>>> 
>>>>> When the JDK is promoted, the code to remove
>>>>> ArrayKlass::_component_mirror will be changed under a new bug id.
>>>>> 
>>>>> http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
>>>>> 
>>>>> Finally, a compatibility request and licensee notification will occur to
>>>>> remove the function JVM_GetComponentType.
>>>>> 
>>>>> Performance testing was done that shows no difference in performance.
>>>>> The isArray() call is a compiler intrinsic which is now called instead
>>>>> of getComponentType, which was recognized as a compiler intrinsic.
>>>>> 
>>>>> JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8
>>>>> tests were performed on both the change requested (1st one) and the full
>>>>> change.
>>>>> 
>>>>> hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
>>>>> 
>>>>> Please send your comments.
>>>>> 
>>>>> Thanks,
>>>>> Coleen
>>> 
>> 
>