RFR (S): 8067438: Add test to verify minimal heap size

[Lindenmaier, Goetz]

Hi Bengt,

there is wb.getVMPageSize().
(see  test/runtime/memory/ReadVMPageSize.java)
But I think the math will be quite complex, I would assume that it depends on 
the gc used.

So if the test still catches the cases you intended to check, I would
prefer the '>=' tests.

And yes, using -Xmx2m would make sense then.

Should I make a webrev?

Best regards,
  Goetz.




-----Original Message-----
From: Bengt Rutisson [mailto:bengt.rutisson at oracle.com] 
Sent: Mittwoch, 7. Januar 2015 13:18
To: Lindenmaier, Goetz
Cc: hotspot-gc-dev at openjdk.java.net
Subject: Re: RFR (S): 8067438: Add test to verify minimal heap size


Hi Goetz,

On 2015-01-07 12:51, Lindenmaier, Goetz wrote:
> Hi Bengt,
>
> Your new tests fails on our ppc machine which has 64K default page size
> (internally we  have an ia64 with 16K pages, will fail there too later on) :
>    JavaTest Message: Test threw exception: java.lang.RuntimeException: (assert failed: 33554432 == 4194304)
>
> According to your comment in the test this was to be expected.
> I already fixed a row of issues with the bigger pages (see
> http://cr.openjdk.java.net/~goetz/webrevs/8067941-64K/webrev.01/)

Right. With a 64k page size you get the heap size aligned up to 32m even 
though you specify 4m on the command line.

> The question is how to fix this.

Yes, agreed.

> I could increase the -Xmx argument to something that causes the test to pass,
> which then isn't really anymore testing for small heaps.

Exactly. The reason for adding the test was that we need a test that run 
with a very small heap size since it has proven useful to find bugs in 
our heap setup code. Changing the test to use -Xmx32m will make it more 
or less useless.

> Or I could change the assert to test for >=
>          assertTrue(Long.parseLong(maxHeap) >= Long.parseLong(expectedMaxHeap),
>                     "Max heap is smaller (" + maxHeap + ") than requested by MaxHeapSize (" + expectedMaxHeap + ").");

This seems like a simple workaround for now. With that change we could 
probably also change to use -Xmx2m instead of -Xmx4m.

A more proper way of fixing this would be to add a whitebox API call to 
get the actual page size and then calculate what the expectedMaxHeap 
size should be.

Since this is a simple test and the issues we have found with small 
heaps before have mostly been crashes during initialization when you 
have a small heap I think your proposal to make the test a bit more 
fuzzy is more appealing than adding a whitebox dependency. What do you 
think?

Thanks,
Bengt

>
> Any recommendations?
>
> Thanks,
>    Goetz.
>
>
>
>
> -----Original Message-----
> From: hotspot-gc-dev [mailto:hotspot-gc-dev-bounces at openjdk.java.net] On Behalf Of Dmitry Fazunenko
> Sent: Mittwoch, 17. Dezember 2014 12:33
> To: Bengt Rutisson
> Cc: hotspot-gc-dev at openjdk.java.net
> Subject: Re: RFR (S): 8067438: Add test to verify minimal heap size
>
> Hi Bengt,
>
>   > https://bugs.openjdk.java.net/browse/JDK-8067768
>   > https://bugs.openjdk.java.net/browse/JDK-8067770
>
> Thanks for that! I submitted corresponding RFEs for new tests.
>
> -- Dima
>
> On 17.12.2014 12:15, Bengt Rutisson wrote:
>> Hi Dima,
>>
>> On 2014-12-16 15:53, Dmitry Fazunenko wrote:
>>> Hi Bengt,
>>>
>>> I completely agree with your approach.
>>> New version of test looks good.
>>> It doesn't cover the boundary case of 2m, but 4m is small enough. We
>>> need separate tests for testing boundary values.
>> Thanks for the review!
>>
>>> BTW, have you submitted bugs for incorrect interpretation of values?
>> I wanted to hear your thoughts before I filed the bug reports. Did it
>> now.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8067768
>> https://bugs.openjdk.java.net/browse/JDK-8067770
>>
>> Thanks,
>> Bengt
>>
>>> Thanks
>>> Dima
>>>
>>>
>>>
>>>
>>> On 16.12.2014 14:16, Bengt Rutisson wrote:
>>>> Hi Dima,
>>>>
>>>> Thanks for looking at this!
>>>>
>>>> On 2014-12-15 15:06, Dmitry Fazunenko wrote:
>>>>> Hi Bengt,
>>>>>
>>>>> The test looks good, but summary needs to be updated.
>>>>>
>>>>>    28  * @summary Verify that the heap gets set up to the expected size
>>>>>
>>>>>  From this summary it's not clear, that the test is for the minimal
>>>>> supported Xmx value.
>>>> Good point. I updated the summary, but I also changed the test a
>>>> bit. See below.
>>>>> Would it make more tests to for minimal heap size?
>>>>> - setting -Xmx from 1024k to 2047k is equivalent to setting 2m.
>>>>> - vm doesn't start if Xmx1023k and less
>>>> You point out a rather strange behavior. The reason the VM does not
>>>> start with 1023K is actually not that we check the maximum heap size
>>>> (Xmx) but that we check the initial heap size (Xms). Xms must be
>>>> larger than 1m otherwise the VM does not start. According to the
>>>> specification of -Xmx it has to be at least 2m:
>>>>
>>>> https://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html
>>>>
>>>> ...but we don't check that. We just silently increase the heap size
>>>> from 1m to 2m and start the VM if you run with -Xmx1m. I find this
>>>> more of a bug than a feature we want to test. So, I prefer to file a
>>>> bug report against that behavior instead of including this in the test.
>>>>
>>>> Another interesting aspect is that the max heap size is aligned to
>>>> fill up the memory that is covered by the card table we set up. We
>>>> size the card table to be aligned with the os page size. Each byte
>>>> in the card table corresponds to 512 bytes of heap memory. This
>>>> means that if we have 4K pages, each pages committed for the card
>>>> table corresponds to 2m of heap. But if we have 8K pages one card
>>>> table page will correspond to 4m of heap. Essentially this means
>>>> that the heap is aligned to 2m or 4m based on the minimal os page size.
>>>>
>>>> On most platforms the minimum page size is 4k but on Sparc it is 8k.
>>>> So, the test I suggested in webrev.00 actually fails on Sparc.
>>>>
>>>> Again I think this is a strange behavior that I'd rather consider a
>>>> bug than a behavior we want to verify in a test.
>>>>
>>>> So, my suggestion is to file two bugs for these issues and instead
>>>> of testing the minimum heap size according to the specification I'll
>>>> just test that a small heap works. If I use 4m for the test it
>>>> should work on all our supported platforms. What do you think about
>>>> this approach?
>>>>
>>>> Here's an updated webrev with a test that uses 4m. Note that the
>>>> test changed its name to TestSmallHeap.
>>>>
>>>> http://cr.openjdk.java.net/~brutisso/8067438/webrev.01/
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>>> Thanks,
>>>>> Dima
>>>>>
>>>>>
>>>>> On 15.12.2014 16:19, Bengt Rutisson wrote:
>>>>>> Hi everyone,
>>>>>>
>>>>>> Can I have a couple of reviews for this small test to verify that
>>>>>> the VM starts with a minimum heap size of 2mb?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~brutisso/8067438/webrev.00/
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8067438
>>>>>>
>>>>>> Thanks,
>>>>>> Bengt