Quantcast

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

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Ioi Lam
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

coleen.phillimore

This looks good.  Glad there was an access flag bit left.

http://cr.openjdk.java.net/~iklam/jdk10/8178543-klass-is-shared.v01/src/share/vm/oops/klassVtable.cpp.udiff.html

I'd make it #ifdef ASSERT instead.  I don't think anyone's built
optimized in a long time tho.

Thanks,
Coleen

On 4/13/17 5:30 AM, Ioi Lam 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Calvin Cheung
In reply to this post by Ioi Lam
Hi Ioi,

The fix looks good to me.

thanks,
Calvin

On 4/13/17, 2:30 AM, Ioi Lam 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Ioi Lam
In reply to this post by coleen.phillimore
Hi Calvin & Coleen,

Thanks for the review. I will make the change as Coleen suggested and push.

- Ioi

On 4/13/17 8:15 PM, [hidden email] wrote:

>
> This looks good.  Glad there was an access flag bit left.
>
> http://cr.openjdk.java.net/~iklam/jdk10/8178543-klass-is-shared.v01/src/share/vm/oops/klassVtable.cpp.udiff.html 
>
>
> I'd make it #ifdef ASSERT instead.  I don't think anyone's built
> optimized in a long time tho.
>
> Thanks,
> Coleen
>
> On 4/13/17 5:30 AM, Ioi Lam 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jiangli Zhou
In reply to this post by Ioi Lam
Hi Ioi,

Could you please explain the change in klassVtable.cpp?

Thanks,
Jiangli

> On Apr 13, 2017, at 2:30 AM, Ioi Lam <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jiangli Zhou
In reply to this post by Ioi Lam
Hi Ioi,

Thanks for the details. Do you know how big is the increase of ‘dirty’ pages with the specific writes? If the increase of unsharable pages are small then it's okay. It’s a balance between speed and memory.

While looking for the callers of itableMethodEntry::initialize(), I found klassItable::initialize_with_method(). It doesn’t seem to be called by anyone. Could you please also remove that as part of your change if you haven’t integrated it. Otherwise, I’ll file a separate bug.

Thanks,
Jiangli

> On Apr 13, 2017, at 9:32 PM, Ioi Lam <[hidden email]> 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.
>
> 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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Ioi Lam
On 4/15/17 2:11 AM, Jiangli Zhou wrote:
> Hi Ioi,
>
> Thanks for the details. Do you know how big is the increase of ‘dirty’ pages with the specific writes? If the increase of unsharable pages are small then it's okay. It’s a balance between speed and memory.

Hi Jiangli,

I ran the start-up benchmark in gdb and set a break point at exit, and
check the smaps file:

BEFORE:

801400000-802280000 rw-p 008e4000 fc:01 17360993                        
/tmp/iklam/bench2/cds.old.jsa
Size:              14848 kB
Rss:               14772 kB
Pss:               14772 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:       228 kB
Private_Dirty:     14544 kB
Referenced:        14772 kB
Anonymous:         14544 kB

AFTER:

801400000-802280000 rw-p 008e4000 fc:01 17360995                        
/tmp/iklam/bench2/cds.new.jsa
Size:              14848 kB
Rss:               14772 kB
Pss:               14772 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:       228 kB
Private_Dirty:     14544 kB
Referenced:        14772 kB
Anonymous:         14544 kB

As you can see, almost the entire RW section is dirty, and there's no
difference with or without the change. So I think the change is pretty safe.

> While looking for the callers of itableMethodEntry::initialize(), I found klassItable::initialize_with_method(). It doesn’t seem to be called by anyone. Could you please also remove that as part of your change if you haven’t integrated it. Otherwise, I’ll file a separate bug.

Thanks for finding the dead code, I will take that out.

Thanks
- Ioi

> Thanks,
> Jiangli
>
>> On Apr 13, 2017, at 9:32 PM, Ioi Lam <[hidden email]> 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.
>>
>> 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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Jiangli Zhou
Hi Ioi,

> On Apr 14, 2017, at 5:39 PM, Ioi Lam <[hidden email]> wrote:
>
> On 4/15/17 2:11 AM, Jiangli Zhou wrote:
>> Hi Ioi,
>>
>> Thanks for the details. Do you know how big is the increase of ‘dirty’ pages with the specific writes? If the increase of unsharable pages are small then it's okay. It’s a balance between speed and memory.
>
> Hi Jiangli,
>
> I ran the start-up benchmark in gdb and set a break point at exit, and check the smaps file:
>
> BEFORE:
>
> 801400000-802280000 rw-p 008e4000 fc:01 17360993                         /tmp/iklam/bench2/cds.old.jsa
> Size:              14848 kB
> Rss:               14772 kB
> Pss:               14772 kB
> Shared_Clean:          0 kB
> Shared_Dirty:          0 kB
> Private_Clean:       228 kB
> Private_Dirty:     14544 kB
> Referenced:        14772 kB
> Anonymous:         14544 kB
>
> AFTER:
>
> 801400000-802280000 rw-p 008e4000 fc:01 17360995                         /tmp/iklam/bench2/cds.new.jsa
> Size:              14848 kB
> Rss:               14772 kB
> Pss:               14772 kB
> Shared_Clean:          0 kB
> Shared_Dirty:          0 kB
> Private_Clean:       228 kB
> Private_Dirty:     14544 kB
> Referenced:        14772 kB
> Anonymous:         14544 kB
>
> As you can see, almost the entire RW section is dirty, and there's no difference with or without the change. So I think the change is pretty safe.

Thanks for looking into that.

>
>> While looking for the callers of itableMethodEntry::initialize(), I found klassItable::initialize_with_method(). It doesn’t seem to be called by anyone. Could you please also remove that as part of your change if you haven’t integrated it. Otherwise, I’ll file a separate bug.
>
> Thanks for finding the dead code, I will take that out.

Thanks!

Jiangli

>
> Thanks
> - Ioi
>> Thanks,
>> Jiangli
>>
>>> On Apr 13, 2017, at 9:32 PM, Ioi Lam <[hidden email]> 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.
>>>
>>> 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 <[hidden email]> 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
>

Loading...