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

Bengt Rutisson bengt.rutisson at oracle.com
Wed Sep 25 23:22:57 PDT 2013


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
>>>>



More information about the hotspot-gc-dev mailing list