RFR[XS] JDK-8178543 - Optimize Klass::is_shared()

[Ioi Lam]

Hi Jiangli,

Code like this in klassVtable.cpp is necessary:

             if (is_preinitialized_vtable()) {   /// calls 
_klass->is_shared()
               // At runtime initialize_vtable is rerun for a shared class
               // (loaded by the non-boot loader) as part of 
link_class_impl().
               // The dumptime vtable index should be the same as the 
runtime index.
               assert(def_vtable_indices->at(i) == initialized,
                      "dump time vtable index is different from runtime 
index");
             } else {
               def_vtable_indices->at_put(i, initialized); //set vtable 
index
             }

because the def_vtable_indices for a shared class is in RO space and 
cannot written into at run time.

For this code (before)

1017   if (MetaspaceShared::is_in_shared_space((void*)&_method) &&
1018      !MetaspaceShared::remapped_readwrite()) {
1019     // At runtime initialize_itable is rerun as part of 
link_class_impl()
1020     // for a shared class loaded by the non-boot loader.
1021     // The dumptime itable method entry should be the same as the 
runtime entry.
1022     assert(_method == m, "sanity");
1023   } else {
1024     _method = m;
1025   }
1026 }

the address (&_method) is either in a vtable or an itable, both of which 
are in the RO space. Therefore it's OK to write into it.

The cost of checking 
MetaspaceShared::is_in_shared_space((void*)&_method) is high, as at this 
point we don't readily have a pointer to the corresponding Klass whose 
i/v table encompasses (&_method).

Therefore, I made the assignment unconditional. I kept the assert so we 
don't lose any functionality.

I think the cost of writing into _method is low (compared to the cost of 
MetaspaceShared::is_in_shared_space) , because it's within an 
InstanceKlass, which will be written into anyway during 
InstanceKlass::restore_unshareable_info.

AFTER:

1017 #ifdef ASSERT
1018   if (MetaspaceShared::is_in_shared_space((void*)&_method) &&
1019      !MetaspaceShared::remapped_readwrite()) {
1020     // At runtime initialize_itable is rerun as part of 
link_class_impl()
1021     // for a shared class loaded by the non-boot loader.
1022     // The dumptime itable method entry should be the same as the 
runtime entry.
1023     assert(_method == m, "sanity");
1024   }
1025 #endif
1026   _method = m;
1027 }

Thanks
- Ioi

On 4/14/17 2:02 AM, Jiangli Zhou wrote:
> Hi Ioi,
>
> Could you please explain the change in klassVtable.cpp?
>
> Thanks,
> Jiangli
>
>> On Apr 13, 2017, at 2:30 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
>>
>> Hi, please review this small start-up enhancement:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8178543
>> http://cr.openjdk.java.net/~iklam/jdk10/8178543-klass-is-shared.v01/
>>
>> We have a benchmark that shows Klass::is_shared() is called very frequently during InstanceKlass::link_class_impl, and costs about 2% of the start-up time.
>>
>> The fix is simple -- instead of walking the list of CDS shared regions, use a new bit in Klass::_access_flags
>>
>> Thanks
>> - Ioi