RFR(S): 8166910, 8166911: Convert TestNewSize_test and TestOldSize_test to GTest
Kirill Zhaldybin
kirill.zhaldybin at oracle.com
Wed Oct 19 20:55:16 UTC 2016
Jesper,
Thank you for review!
Regards, Kirill
On 19.10.2016 23:45, Jesper Wilhelmsson wrote:
> Looks good!
> /Jesper
>
> Den 19/10/16 kl. 19:49, skrev Kirill Zhaldybin:
>> Jesper,
>>
>> Thank you for reviewing the fix!
>>
>> Here are a new WebRev:
>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166910/webrev.01/
>>
>> Changes:
>> 1. I created a small class MinHeapSizeGuard which stores min_heap_size
>> value and
>> restores it after we leave TestWrapper::test scope.
>> 2. I also changed TEST to TEST_VM since otherwise I got assert on not
>> defined
>> page size in debug build.
>>
>> Could you please let me know your opinion?
>>
>> Thank you.
>>
>> Regards, Kirill
>>
>> On 19.10.2016 16:40, Jesper Wilhelmsson wrote:
>>> Hi Kirill,
>>>
>>> In TestWrapper you modify _min_heap_size, but this variable is not
>>> protected by the FLAG_GUARD macros. (It is not a flag so it can't be.) I
>>> think you need to restore it's value since you run some of your tests in
>>> the same VM as other tests.
>>>
>>> Besides that the test looks OK. I'm not 100% comfortable with the
>>> executor classes, but I don't have to be either :)
>>> /Jesper
>>>
>>>
>>> Den 13/10/16 kl. 21:26, skrev Kirill Zhaldybin:
>>>> Dear all,
>>>>
>>>> Could you please review this fix for 8166910 and 8166911?
>>>>
>>>> Changes:
>>>> 1. All asserts changed to GTest's
>>>> 2. The test separated to 4 scenarios.
>>>> 3. Since the initial internal test could (and actually did) impact
>>>> other tests I
>>>> moved test scenarios which use FLAG_SET_CMDLINE to other vm. Otherwise
>>>> even
>>>> after successful execution running the same test again caused failure.
>>>> 4. Flag storage changed to new nice FLAG_GUARD macros.
>>>> 5. The test deeply refactored from straight scenarios to the series of
>>>> executors
>>>> and test wrapper which runs them.
>>>>
>>>> WebRev:
>>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166910/webrev.00/
>>>>
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8166910
>>>> https://bugs.openjdk.java.net/browse/JDK-8166911
>>>>
>>>> Thank you.
>>>>
>>>> Regards, Kirill
>>>>
>>>>
>>
More information about the hotspot-gc-dev
mailing list