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

[Doug Simon]

> Date: Tue, 01 Jul 2014 23:26:43 -0700
> From: Mandy Chung <mandy.chung at oracle.com>
> To: Coleen Phillimore <coleen.phillimore at oracle.com>
> Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>,
> 	core-libs-dev <core-libs-dev at openjdk.java.net>
> Subject: Re: RFR 8047737 Move array component mirror to instance of
> 	java/lang/Class
> Message-ID: <53B3A623.1050804 at oracle.com>
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
> 
> 
> On 6/30/2014 9:51 PM, Christian Thalinger wrote:
>> On Jun 30, 2014, at 5:50 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?
>>> Hi,  I've decided to make them consistent and add another parameter to the Class constructor.
>>> 
>>> http://cr.openjdk.java.net/~coleenp/8047737_jdk_2/
> 
> The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
> 
> There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc.  I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added.  Handle it (and other places in JDK) when such JIT optimization comes.

I think doing the assignment to a non-null value in the constructor is the right thing. Do we really want to keep expanding code like this in ciField.cpp:

    KlassHandle k = _holder->get_Klass();
    assert( SystemDictionary::System_klass() != NULL, "Check once per vm");
    if( k() == SystemDictionary::System_klass() ) {
      // Check offsets for case 2: System.in, System.out, or System.err
      if( _offset == java_lang_System::in_offset_in_bytes()  ||
          _offset == java_lang_System::out_offset_in_bytes() ||
          _offset == java_lang_System::err_offset_in_bytes() ) {
        _is_constant = false;
        return;
      }
    }

This code may also need to be duplicated and maintained for other JIT compilers that don’t use the C++ compiler interface.

In practice at the moment, I don’t think it makes a difference either way. Apart from @Stable fields, I think non-static fields with a null value are not treated as constant (by all JITs I know of).

-Doug