RFR(L): 8195097: Make it possible to process StringTable outside safepoint

[Ioi Lam]

Hi Robin,

I've looked at the changes related to CDS. It looks OK to me but I have 
the following comments:

One problem I found with reviewing stringTable.cpp is that some of the 
functions are moved around, which means it's hard to match the "before" 
and "after" code in the diff. For example: StringTable::verify has been 
moved to below StringTable::dump().

Would it be possible for you to rearrange the new code to minimize the 
size of the diffs, so it's easier to see what has been changed?

Also, (although this was an issue in the original code), could you 
rename StringTable::copy_shared_string to 
StringTable::copy_shared_string_table? This function copies not just 
than one string, but the entire table.

Thanks

- Ioi


On 5/29/18 2:13 AM, Robbin Ehn wrote:
> Hi,
>
> For reference here is the ZGC patch:
> http://mail.openjdk.java.net/pipermail/zgc-dev/2018-May/000354.html
>
> I removed two methods I had left in for an older ZGC patch and fixed 
> some minors in stringTable.hpp.
> Inc: http://cr.openjdk.java.net/~rehn/8195097/v1/inc/webrev/
> Full:http://cr.openjdk.java.net/~rehn/8195097/v1/full/webrev/
>
> /Robbin
>
> On 05/28/2018 03:19 PM, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> This implements the StringTable with the ConcurrentHashtable for 
>> managing the
>> strings using oopStorage for backing the actual oops via WeakHandles.
>>
>> The unlinking and freeing of hashtable nodes is moved outside the 
>> safepoint,
>> which means GC only needs to walk the oopStorage, either concurrently 
>> or in a
>> safepoint. Walking oopStorage is also faster so there is a good 
>> effect on all
>> safepoints visiting the oops.
>>
>> The unlinking and freeing happens during inserts when dead weak oops are
>> encountered in that bucket. In any normal workload the stringtable 
>> self-cleans
>> without needing any additional cleaning. Cleaning/unlinking can also 
>> be done
>> concurrently via the ServiceThread, it is started when we have a high 
>> ‘dead
>> factor’. E.g. application have a lot of interned string removes the 
>> references
>> and never interns again. The ServiceThread also concurrently grows 
>> the table if
>> ‘load factor’ is high. Both the cleaning and growing take care to not 
>> prolonging
>> time to safepoint, at the cost of some speed.
>>
>> Kitchensink24h, multiple tier1-5 with no issue that I can relate to this
>> changeset, various benchmark such as JMH, specJBB2015.
>>
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8195097
>> Webrev: http://cr.openjdk.java.net/~rehn/8195097/v0/webrev/
>>
>> Thanks, Robbin