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

[Gerard Ziemski]

hi Robbin, Coleen,

I apologize, but in my haste to try to push this issue before I went off for holiday break (which I failed), I missed another issue, so I’d like to ask you to review this one last small change from webrev2:

line 129 in jdk/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp should be 

129     cht_insert_helper(thr, cht, val);

instead of

129     cht_get_helper(thr, cht, val);

bug id:  https://bugs.openjdk.java.net/browse/JDK-8215155
webrev:  http://cr.openjdk.java.net/~gziemski/8215155_rev3
testing: Passed Mach hs_tier1,2,3


thank you

> On Dec 13, 2018, at 9:48 AM, Gerard Ziemski <gerard.ziemski at oracle.com> 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 posted http://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…
> 
> 
> cheers