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

[Gerard Ziemski]

Looks good.

Small nitpick: the usage of white space in “internal_get_insert” and “ internal_get” could be more consistent.

Also, you say the performance is “different, not better or worse”, so I assume you mean no significant perf regressions (or benefit) to report?

P.S. I created a clone JDK-8214310 to do the same for the SymbolTable.


cheers

> On Nov 20, 2018, at 7:43 AM, Robbin Ehn <robbin.ehn at oracle.com> 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