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-gc-dev
mailing list