(new patch) RFR(s): 8213791: StringTable: Use get and insert

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~rehn/8213791/webrev/src/hotspot/share/classfile/stringTable.cpp.frames.html

365 do {


Can you add a comment about why it's a loop:
// This may be concurrent with another insert.  Loop until we either 
insert the string or get the string
// that the other thread inserted.  Insert returns false if another 
thread beat us to it.

Or something like that?

Everything else looks good.  internal_get_insert can be removed when the 
similar change is made for the SymbolTable, right?

I don't need to see another webrev for the new comment.

Thanks,
Coleen


On 11/20/18 8:43 AM, Robbin Ehn wrote:
> Hi all, please review.
>
> Webrev:
> http://cr.openjdk.java.net/~rehn/8213791/webrev/
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8213791
>
> I created an plain(:er) insert which always allocate the Node outside.
> This makes the critical section shorter, which is good for overall 
> performance.
> But a bit slower in the contended bucket case.
> So performance is different, not better or worse.
>
> If we can move SymbolTable to the same logic, we can remove the 
> get_insert.
>
> Stress test stringtable, t1-3 and gtests passes.
>
> Thanks, Robbin
>
> On 11/13/18 6:21 PM, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> It's problematic with allocating inside a critical section.
>> - You are not allowed to safepoint.
>> - Allocate cannot use GlobalCounter write side for lock free 
>> allocations.
>> - Easy to accidentally create deadlocks.
>> This patch moves the allocation for oopStorage outside the hashtable.
>> Performance is unaffected, if you don't hammer StringTable with 
>> multiple threads
>> using identical strings.
>>
>> This is just a first small step, mainly for not limiting oopStorage.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rehn/8213791/webrev/
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8213791
>>
>> Parent CR: ConcurrentHashTable: Do not allocate inside critical section
>> https://bugs.openjdk.java.net/browse/JDK-8209054
>>
>> Passes t1-3, multiple thread same string intern manual test and jmh 
>> stress test.
>>
>> Thanks, Robbin