RFR(S) 8246181: Avoid walking SymbolTable during -Xshare:dump

[Calvin Cheung]

On 6/24/20 10:08 AM, Yumin Qi wrote:
>
> HI, Calvin
>
>
> Thanks for review!
>
> On 6/24/20 9:18 AM, Calvin Cheung wrote:
>> Hi Yumin,
>>
>> Code changes look good.
>>
>> I guess the following assert isn't applicable to the new code anymore 
>> since the SymbolTable::do_add_if_needed will be called for 
>> non-permanent symbols?
>>
>> 1244     assert((*sym)->is_permanent(), "archived symbols must be 
>> permanent");
>>
>> Are we archiving more symbols with this change?
>>
> It does not change the number of symbols, in development, I already 
> checked with original result with the new change. In -Xshare:dump, all 
> symbols are permanent, I could add this back.
>
> + for (int i = 0; i < all_symbols->length(); i++) { + 
> assert(all_symbols->at(i)->is_permanent(), "archived symbols must be 
> permanent"); // <---- add here new line metaspaceShared.cpp: 1460 + 
> all_symbols->at(i)->update_identity_hash();
>         }
>
> Let me know if you need second webrev, or I will push with this adding.

No need for another webrev. Do some testing with the assert before push.

thanks,

Calvin

>
> Thanks
> Yumin
>> thanks,
>>
>> Calvin
>>
>> On 6/23/20 2:44 PM, Yumin Qi wrote:
>>> Hi, Please review
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8246181
>>>
>>> Webrev:http://cr.openjdk.java.net/~minqi/2020/8246181/webrev-00/
>>>
>>>
>>> Summary: This is a little different from the bug suggested 
>>> implementation. To avoid working the SymbolTable during dump, the 
>>> symbol is added to a global array when it is created, 
>>> SymbolTable::do_add_if_needed. The symbol only created once during 
>>> its lifetime. There is just little performance overhead for runtime.
>>>
>>>
>>> Tests: local jtreg on runtime/cds/appcds for debug/release.
>>>
>>> mach5: tier1,tier2,tier4
>>>
>>>
>>> Thanks
>>>
>>> Yumin
>>>