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

[coleen.phillimore at oracle.com]

On 1/9/19 2:48 PM, Gerard Ziemski wrote:
> 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);

Makes sense.  This looks good.  You can run the gtests locally on your 
machine, which is a lot faster.

Coleen
>
> 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