RFR(S): 8164738: Convert AltHashing_test to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Tue Aug 30 12:13:23 UTC 2016


Coleen,

Thank you for review!

Regards, Kirill

On 29.08.2016 23:44, Coleen Phillimore wrote:
>
> This test conversion looks good.
>
> On 8/29/16 11:06 AM, Kirill Zhaldybin wrote:
>> David,
>>
>> Thank you for review!
>>
>>
>> On 26.08.2016 06:38, David Holmes wrote:
>>> On 25/08/2016 1:47 AM, Kirill Zhaldybin wrote:
>>>> Dear all,
>>>>
>>>> Could you please review this fix for 8164738?
>>>
>>> Seems okay.
>>>
>>>> To convert the test I added new friend class to AltHashing class so we
>>>> could access private member function static juint murmur3_32(const 
>>>> int*
>>>> data, int len). There are also few formating fixes.
>>>
>>> Any reason all the murmur functions shouldn't be public? I'm not a 
>>> fan of friends. No big deal either way.
>> Well, I am not an author so I could only speculate that if
>> static juint murmur3_32(const int* data, int len);
>> static juint murmur3_32(juint seed, const int* data, int len);
>>
>> are used only from AltHashing class according to general "the less 
>> visible the better" rule they were made private.
>>
>
> Yes, that's why the functions are private.  In general, the tests 
> should probably be made friends if they're going to use private 
> functions rather than making the functions public for the rest of the 
> JVM to use.
>
> thanks,
> Coleen
>
>> Regards, Kirill
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> WebRev: 
>>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164738/webrev.00/
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8164738
>>>>
>>>> Regards, Kirill
>>
>



More information about the hotspot-runtime-dev mailing list