RFR(S): 8166462: Convert TestResourcehash_test to Gtest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Thu Sep 29 20:14:03 UTC 2016


Igor,

Thank you for review!

Regards, Kirill

On 09/29/2016 09:46 PM, Igor Ignatyev wrote:
>> Here are a new WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166462/webrev.01/
> looks good to me.
>
> — Igor
>> On Sep 28, 2016, at 6:47 PM, Kirill Zhaldybin <kirill.zhaldybin at oracle.com> wrote:
>>
>> Mikael,
>>
>> Thank you for reviewing the fix!
>>
>> Here are a new WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166462/webrev.01/
>> I switched to using static method calls instead of redefined operator().
>>
>> Could you please let me know your opinion?
>>
>> Thank you.
>>
>> Regards, Kirill
>>
>> On 28.09.2016 16:27, Mikael Gerdin wrote:
>>> Hi Kirill,
>>>
>>> On 2016-09-26 23:19, Kirill Zhaldybin wrote:
>>>> Dear all,
>>>>
>>>> Could you please review this fix for 8166462?
>>>>
>>>> After discussion with the author of the original test (Mikael Gerdin) a
>>>> few changes were done during conversion:
>>>> 1. removed "if not product" guards and checks for Resource Marks since
>>>> we believe that we are in controlled env and we cannot have any RM
>>>> 2. parametrized "small" test and removed "shifted small" test since they
>>>> were very similar and depended only on one parameter
>>>>
>>>> WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166462/webrev.00/
>>> I think this change is overall ok but I'm not too fond of using operator().
>>> I'd prefer if you kept the original variant of calling a static method on class Runner instead.
>>>
>>> Something like:
>>> 63 class SmallResourceHashtableTest : public CommonResourceHashtableTest {
>>>   64  protected:
>>>   65
>>>   66   template<
>>>   67   unsigned (*HASH) (K const&) = primitive_hash<K>,
>>>   68   bool (*EQUALS)(K const&, K const&) = primitive_equals<K>,
>>>   69   unsigned SIZE = 256,
>>>   70   ResourceObj::allocation_type ALLOC_TYPE = ResourceObj::RESOURCE_AREA
>>>
>>>   71   >
>>>   72   class Runner : public AllStatic {
>>>
>>>
>>>   73    public:
>>>   74
>>>   75     static void test(V step) {
>>> ...
>>>
>>> The class is only there to allow template parameters with default values anyway.
>>>
>>> /Mikael
>>>
>>>> CR:https://bugs.openjdk.java.net/browse/JDK-8166462
>>>>
>>>> Thank you.
>>>>
>>>> Regards, Kirill




More information about the hotspot-runtime-dev mailing list