RFR (XS): 8234127: BasicHashtable does not support small table_size

[coleen.phillimore at oracle.com]

On 11/14/19 7:28 PM, Ioi Lam wrote:
>
>
> On 11/14/19 1:47 PM, Man Cao wrote:
>> Thanks for the review.
>>
>>> It's not directly related, I'm wondering why such small hashtable is
>>> needed in your use case.
>> The test G1AddMetaspaceDependency.java sets -XX:G1UpdateBufferSize=1, 
>> which
>> would create a size-one hashtable with my pending change for 
>> JDK-8087198.
>> Real users should probably not set G1UpdateBufferSize so small, 
>> unless they
>> want to stress G1's code for concurrent refinement.
>>
>> -Man
>
> If a minimum size of 1 doesn't make sense, maybe in your patch for 
> JDK-8087198, you can update the allowable range in globals.hpp, or 
> manually override it to a minimum size that makes sense? That can 
> prevent incorrect settings that would cause unintended slow down.
>
>   product(size_t, G1UpdateBufferSize, 
> 256,                                  \
>           "Size of an update 
> buffer")                                       \
>           range(1, NOT_LP64(32*M) 
> LP64_ONLY(1*G))                           \
> \
>

Yes, this should be done also.
Coleen

> Thanks
> - Ioi
>
>>
>> On Thu, Nov 14, 2019 at 1:20 PM Jiangli Zhou <jianglizhou at google.com> 
>> wrote:
>>
>>> Hi Man,
>>>
>>> Just took a look. Looks fine to me as well.
>>>
>>> It's not directly related, I'm wondering why such small hashtable is
>>> needed in your use case.
>>>
>>> Thanks,
>>> Jiangli
>>>
>>> On Thu, Nov 14, 2019 at 11:19 AM Man Cao <manc at google.com> wrote:
>>>> Thanks for the review. Yes, I will try using ResourceHashtable in new
>>> code.
>>>> The BasicHashtable does not work with size 2 and 3, either. In my use
>>> case,
>>>> the initial size is based on a JVM flag (G1UpdateBufferSize), so it is
>>>> dependent on user input.
>>>>
>>>> -Man
>>>>
>>>>
>>>> On Thu, Nov 14, 2019 at 10:52 AM <coleen.phillimore at oracle.com> wrote:
>>>>
>>>>> This fix seems fine, but having a hashtable with a starting length 1
>>>>> seems silly.   Unless I'm reading this wrong.   As Ioi wrote in his
>>>>> comment, there might be a better hashtable for your work.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>> On 11/13/19 8:00 PM, Man Cao wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Can I have reviews for this small bug fix?
>>>>>> Webrev: https://cr.openjdk.java.net/~manc/8234127/webrev.00/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234127
>>>>>>
>>>>>> I'm trying to make use of KVHashtable in JDK-8087198 and encountered
>>> this
>>>>>> bug.
>>>>>>
>>>>>> -Man
>>>>>
>