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

Gerard Ziemski gerard.ziemski at oracle.com
Thu Dec 13 15:48:58 UTC 2018


> 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





More information about the hotspot-runtime-dev mailing list