RFR(S): 8159818: Convert IHOP_test to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Tue Sep 27 08:53:38 UTC 2016


Igor,

Thank you for review!

Regards, Kirill

On 09/27/2016 11:47 AM, Igor Ignatyev wrote:
> Kirill,
>
> the fix looks good to me.
>
> Thanks,
> — Igor
>
>> On Sep 26, 2016, at 5:55 PM, Kirill Zhaldybin <kirill.zhaldybin at oracle.com> wrote:
>>
>> Thomas,
>>
>> Thank you for reviewing the fix!
>>
>> Here are a new WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8159818/webrev.01/
>>
>> Could you please let me know your opinion?
>>
>> Changes:
>> 1. g1IHOPControl.hpp copyright updated
>> 2. static void test_update parameters list reformatted.
>> 3. Comment "Test could be only run with G1" changed to "Test requires G1"
>> 4. Some reviewers are pretty strict about 80 symbols. Could you please let me know if I need to change limit for the line lengths?
>>
>> Thank you.
>>
>> Regards, Kirill
>>
>> On 26.09.2016 11:33, Thomas Schatzl wrote:
>>> Hi Kirill,
>>>
>>> On Thu, 2016-09-22 at 22:38 +0300, Kirill Zhaldybin wrote:
>>>> Dear all,
>>>>
>>>> Could you please review this fix for 8159818?
>>>>
>>>> Two tests were converted to GTest.
>>>> Since these tests require G1 GC, checks were added to prevent them to
>>>> be
>>>> run with other GCs.
>>>> Comments "// @requires UseG1GC" were added to facilitate finding such
>>>> tests.
>>>>
>>>> WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8159818/webr
>>>> ev.00/
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8159818
>>>    - g1IHOPControl.hpp needs copyright update
>>>
>>> test_g1IHOPControl.cpp:
>>>
>>> -   30 static void test_update(G1IHOPControl* ctrl, double alloc_time,
>>>      31         size_t alloc_amount, size_t young_size, double
>>> mark_time) {
>>>
>>> please follow usual hotspot style guidelines for formatting parameter
>>> lists.
>>>
>>> -   40   // Test could be only run with G1
>>>      75   // Test could be only run with G1
>>>
>>> -> "Test requires G1."
>>>
>>> -   95   const size_t settled_ihop1 = target_size
>>>    96           - (young_size + alloc_amount1 / alloc_time1 *
>>> marking_time1);
>>>
>>> when breaking lines (the whole file seems to have a strict ~80 chars
>>> line limit, which we do not require), I prefer to have the next line
>>> not begin with an operator (others may disagree, so ignore this if
>>> wanted).
>>> Anyway I think there is no need to limit the line lengths to 80 chars.
>>>
>>> Looks good otherwise.
>>>
>>> Thanks
>>>    Thomas
>>>




More information about the hotspot-gc-dev mailing list