RFR 8215155: Remove get_insert() from concurrent hashtable and gtests

[Robbin Ehn]

Thanks for fixing Gerard!

/Robbin

On 12/17/18 10:44 PM, coleen.phillimore at oracle.com wrote:
> 
> 
> On 12/13/18 10:48 AM, Gerard Ziemski wrote:
>>> On Dec 13, 2018, at 8:58 AM, Gerard Ziemski<gerard.ziemski at oracle.com>  wrote:
>>>
>>>
>>>> On Dec 12, 2018, at 6:30 PM,coleen.phillimore at oracle.com  wrote:
>>>>
>>>>
>>>> This looks good to me.
>>>>
>>>> On 12/12/18 3:01 PM, Gerard Ziemski wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this enhancement followup to JDK-8214310 and JDK-8213791
>>>>>
>>>>> Here we redo the relevant gtest tests to use get(),insert() and are now free to remove the get_insert() API from the concurrent hash table implementation.
>>>>>
>>>>> bug id:https://bugs.openjdk.java.net/browse/JDK-8215155
>>>>> webrev:http://cr.openjdk.java.net/~gziemski/8215155_rev1
>>>>> Testing: Passed Mach hs_tier1, another Mach hs_tier1,2,3,4,5,6 in progress…
>>>>>
>>>>> P.S. I’d like to push this into JDK12 if there is no contention about this fix…
>>>> It seems pretty straightforward.  I'm fine with your pushing this to 12 but the deadline is fast approaching and I think you should have another reviewer.
>>> Thank you for the review Coleen!
>>>
>>> As this is a test related change plus trivial removal of unused API, normally I’d mark it as trivial, but I agree with you - a second reviewer here would be most appreciated.
>> I just discovered that I had an assert reversed in my webrev1 fix - in the file jdk/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp, line 108:
>>
>>   EXPECT_EQ(cht->insert(thr, stl, val), false) << "Inserting an unique value failed.”;
>>
>> should be:
>>
>>   EXPECT_EQ(cht->insert(thr, stl, val), true) << "Inserting an unique value failed.”;
>>
>> I fixed it and postedhttp://cr.openjdk.java.net/~gziemski/8215155_rev2
>>
>> I fired another Mach5 test (why wasn’t it caught?), but now I feel a bit less confident about pushing it into jdk12…
> 
> I can't explain that either.  You can run locally via *make test-hotspot-gtest
> *and with some more options to just run the test.
> 
> I read it a bit more carefully this time.  It looks good.
> 
> Coleen
> **
>> cheers
>>
>>
>>
>