RFR (XS): 8234127: BasicHashtable does not support small table_size
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Fri Nov 15 01:06:32 UTC 2019
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
>>>>>
>
More information about the hotspot-runtime-dev
mailing list