RFR(S): 8159818: Convert IHOP_test to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Mon Sep 26 14:55:13 UTC 2016


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