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