RFR (S): 8023158 - hotspot/test/gc/7168848/HumongousAlloc.java fails 14 full gcs, expect 0 full gcs

[Bengt Rutisson]

Hi Per,

Looks good.

One very minor nit. I'd prefer the output from the test to go to 
System.out rather than System.err. If you agree you don't have to 
generate a new webrev for that change.

Thanks,
Bengt

On 9/25/13 10:11 PM, Per Liden wrote:
> Hi Tao,
>
> Yes, it's moved and you'll see that it says "(was test/gc/7168848/HumongousAlloc.java)" on the index page.
>
> /Per
>
> On Sep 25, 2013, at 19:07, Tao Mao <tao.mao at oracle.com> wrote:
>
>> Hi Per,
>>
>> Did you run 'hg rename foo bar' to actually rename the test file? I didn't see this info in .patch file.
>>
>> Thanks.
>> Tao
>>
>> On 9/24/13 6:33 AM, Per Liden wrote:
>>> Hi Thomas,
>>>
>>> Thanks for reviewing. Updated webrev here:
>>> http://cr.openjdk.java.net/~pliden/8023158/webrev.02/
>>>
>>> Comments inline.
>>>
>>> On 2013-09-24 14:49, Thomas Schatzl wrote:
>>>> Hi,
>>>>
>>>> On Mon, 2013-09-23 at 18:13 +0200, Per Liden wrote:
>>>>> Hi!
>>>>>
>>>>> http://cr.openjdk.java.net/~pliden/8023158/webrev.01/
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8023158
>>>>>
>>>>> Summary: A side effect of fixing JDK-8020155 was that the HumongousAlloc
>>>>> test for G1 broke. This test is designed to make sure than an allocation
>>>>> of an humongous object initiates a mark phase if that allocation pushes
>>>>> the heap size above InitiatingHeapOccupancyPercent. Due to the changes
>>>>> made in JDK-8020155 this test fails since it instead provokes Full GCs.
>>>>> This patch fixes the test. Also renamed the test according to the new
>>>>> naming schema.
>>>> - the name of the class does not correspond to the java file name, so it
>>>> does not compile (probably a last-minute change).
>>> Moved the file last minute... doh!
>>>
>>>> - one suggestion for the test: please assign the dummy LOB to a global
>>>> static instead of a local variable. The compiler can remove that
>>>> allocation easily, say when run with -Xcomp and the server compiler. I
>>>> tried adding "-Xcomp" and "-server" to the invocation of the
>>>> ProcessBuilder and it did not fail though. It may not hurt to do that
>>>> anyway.
>>> Yep, will do.
>>>
>>>> One personal style comment: not sure if you need to prefix all variables
>>>> in the test with "g1", but that's just my opinion. I am fine either way.
>>> I'll drop those.
>>>
>>> cheers,
>>> /Per
>>>
>>>> Thanks,
>>>> Thomas
>>>>