RFR(S): 8204857: ConcurrentHashTable: Fix parallel processing

Robbin Ehn robbin.ehn at oracle.com
Fri Jun 15 18:27:38 UTC 2018


Hi Gerard,

On 2018-06-15 17:31, Gerard Ziemski wrote:
> hi Robbin,
> 
> Looks OK.

Thanks,

> 
> Two questions though
> 
> #1 what is the purpose of the code (line 1003):
> 
>      for (uintptr_t v = 1; v < 99999; v++ ) {
>        TestLookup tl(v);
>        cht->get_copy(this, tl);
>      }
> 
> Is it to “interfere” with the multithreaded delete operation? If so, I guess we can’t put any asserts around "cht->get_copy(this, tl);” as some items might be deleted and some not at that point?

Yes, if we have a bug I'm hoping that this reader will crash, as long as 
get_copy return without crashing we are happy.

> 
> 
> #2 I don’t see how the parallel task split the work to cooperate - is it really useful to have bulk_delete with threads that have the same evaluators? Wouldn’t the threads be competing with each other, rather that dividing the task and cooperating?
> 

They work on different ranges in backing bucket array.
First thread to claim a piece will get 4096 (2^12) buckets and loop over 0 to 
4095. Next thread will claim the second piece at same size but loop 4096->8191 
and so on. The thread will claim a new range when it's finished until entire 
table is done.

As a note, an optimization here would be to try to not let them work on adjacent 
pieces, but instead spread them out to reduce cache collisions.

/Robbin

> 
> 
> cheers
> 
>> On Jun 13, 2018, at 5:48 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
>>
>> On 06/12/2018 08:14 PM, coleen.phillimore at oracle.com wrote:
>>> This looks good.
>>
>> Thanks!
>>
>>> http://cr.openjdk.java.net/~rehn/8204857/webrev/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp.frames.html 1018 EXPECT_TRUE(false) << "Not items should left";
>>> Little typo.
>>
>> Fixed!
>>
>> /Robbin
>>
>>> Thanks,
>>> Coleen
>>> On 6/12/18 10:51 AM, Robbin Ehn wrote:
>>>> Hi, please review,
>>>>
>>>> The parallel processing functionality of the concurrenthastable is not working.
>>>> This adds a gtest for it and fixes the issue. But there are no users of it, so the most important is the gtest so we don't breaks this in the future.
>>>>
>>>> Change-set: http://cr.openjdk.java.net/~rehn/8204857/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204857
>>>>
>>>> /Robbin
> 


More information about the hotspot-runtime-dev mailing list