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

[Ioi Lam]

On 4/14/17 12:32 PM, Ioi Lam wrote:
> 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.
>
Sorry, I meant to say, "both are in the RW space", so it's OK to write.

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