[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S): 8164738: Convert AltHashing_test to GTest
From:       Kirill Zhaldybin <kirill.zhaldybin () oracle ! com>
Date:       2016-08-30 12:13:23
Message-ID: 55146529-b86d-5964-7f4c-b4d7c16a0844 () oracle ! com
[Download RAW message or body]

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
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]