RFR (S): 8026043: add regression test for JDK-8000831
Filipp Zhinkin
filipp.zhinkin at oracle.com
Tue Oct 22 08:40:51 UTC 2013
Hi,
I've done following changes:
- @build tag removed;
- 'garbage' array is now static field;
- added heap-sizing flags to avoid failures with options passed via
'test.java.opts'.
Updated webrev: http://cr.openjdk.java.net/~kshefov/8026043/webrev.02/
Thanks,
Filipp.
On 10/18/2013 07:33 PM, Filipp Zhinkin wrote:
> Hi Erik,
>
> thank you for watching at it!
>
> On 10/18/2013 06:59 PM, Erik Helin wrote:
>> Hi Filipp,
>>
>> thanks for adding new tests, really appreciate it!
>>
>> I have a couple of comments/questions about this test:
>> - Do you really need a @build tag?
> No, I don't.
>> - Could you please provide a comment above each regex with an example
>> that shows what the regex matches?
> Yes, sure.
>> - This test will only run with the default GC (since no GC is
>> specified). What do you think about additional run targets testing
>> various GC combinations?
> The idea was to pass all VM options including different GC's via
> test.java.opts property.
> It makes test more flexible and allow us to test not only some fixed
> subset of GC combinations.
>> - Have you tried running the test with -Xcomp? The reason I'm asking is
>> because I'm afraid that the compiler might be able to realize that
>> the
>> GarbageProducer main method isn't doing anything.
> Yes, I've tried and that object allocation was not optimized out,
> but I agree with you, "garbage" should be static.
>>
>> One thing that usually solves this is having garbage as a public
>> static variable in the class.
>> - Are you using GarbageProducer since you want to trigger young GCs? If
>> so, why only tests young GCs? Would the test works just as well using
>> System.gc() to tests full GCs?
> In fact, test running with MaxHeapSize=5m, so both minor and full GC
> happens.
> But there is another issue then - I forgot to set up other heap
> sizing options,
> so test will fail when someone start if with, for example, -Xms10m.
>
> I'll fix that.
>
> Thanks,
> Filipp.
>> Thanks,
>> Erik
>>
>> On 2013-10-14, Filipp Zhinkin wrote:
>>> Hi,
>>>
>>> I've refactored options parsing, here is updated webrev:
>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.01/
>>>
>>> Note that I'm using ProcessTools.createVerboseJavaProcessBuilder
>>> from 8026047
>>> to make sure that VM output will not be suppressed or redirected to
>>> file.
>>>
>>> Thanks,
>>> Filipp.
>>>
>>> On 10/11/2013 08:22 PM, Tao Mao wrote:
>>>> Ok, thank you for heads-up. Then, I'll await your webrev update.
>>>>
>>>> Thanks.
>>>> Tao
>>>>
>>>> On 10/11/13 9:11 AM, Filipp Zhinkin wrote:
>>>>> Hi Tao,
>>>>>
>>>>> Sorry for typo. You're right, the correct one is
>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.00/.
>>>>>
>>>>> And I'll refactor it in the same way as fix for 8026047.
>>>>>
>>>>> Thanks,
>>>>> Filipp.
>>>>>
>>>>> On 10/11/2013 08:11 PM, Tao Mao wrote:
>>>>>> Hi Filipp,
>>>>>>
>>>>>> FYI, it has a wrong webrev. I guess it should be
>>>>>> http://cr.openjdk.java.net/~kshefov/8026043/webrev.00/
>>>>>>
>>>>>> Thanks.
>>>>>> Tao
>>>>>>
>>>>>> On 10/8/13 8:34 AM, Filipp Zhinkin wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I would like to get couple review on test, that cover issue
>>>>>>> fixed by 8000831 - Heap verification output
>>>>>>> incorrect/incomplete.
>>>>>>>
>>>>>>> Test verifies that options VerifyBeforeGC and
>>>>>>> VerifyAfterGCemit appropriate output if they are turned on
>>>>>>> and never
>>>>>>> emit corrupted output as it was before 8000831 fixed.
>>>>>>>
>>>>>>> Bug ID: https://bugs.openjdk.java.net/browse/JDK-8026043
>>>>>>> Webrev: http://cr.openjdk.java.net/~kshefov/8026047/webrev.00/
>>>>>>> Testing: manual on local host, automated on various platforms.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Filipp.
>
More information about the hotspot-gc-dev
mailing list