RFR (S): 8067438: Add test to verify minimal heap size
Bengt Rutisson
bengt.rutisson at oracle.com
Wed Jan 7 20:07:08 UTC 2015
On 1/7/15 5:04 PM, Lindenmaier, Goetz wrote:
> Hi Bengt,
>
> I had to add the new path to this line:
> * @library /testlibrary /testlibrary/whitebox /../../test/lib
Right. I was using a repository that hadn't been updated to include this
change yet. Good that you noticed it and could handle it.
>
> But then it works fine on the machine with 64K pages.
Great to hear! And my testing on our platforms also passed.
>
> As you now have everything set up, it would be great if
> you could submit this, as I anyways have to find a sponsor.
Sure. No problem. I'll fix it tomorrow.
>
> You could adapt the comment to the new behavior, though.
Absolutely. :)
Thanks,
Bengt
> Thanks!
> Goetz.
>
>
>
> -----Original Message-----
> From: Bengt Rutisson [mailto:bengt.rutisson at oracle.com]
> Sent: Mittwoch, 7. Januar 2015 16:18
> To: Lindenmaier, Goetz
> Cc: hotspot-gc-dev at openjdk.java.net
> Subject: Re: RFR (S): 8067438: Add test to verify minimal heap size
>
>
> On 2015-01-07 16:05, Lindenmaier, Goetz wrote:
>> I'll give it a try ;)
>> I'm anyways at fixing tests.
> Thanks Goetz!
>
> I had to give it a try too. Here's my patch. It seems to work on my
> local workstation. Trying it out in our test system right now. Would be
> great if you could try it with your system too.
>
> If this works and you want to use this patch I'm fine with you sending
> out the webrev. But I can do it too. Your choice.
>
> Bengt
>
> diff --git a/test/gc/TestSmallHeap.java b/test/gc/TestSmallHeap.java
> --- a/test/gc/TestSmallHeap.java
> +++ b/test/gc/TestSmallHeap.java
> @@ -26,10 +26,13 @@
> * @bug 8067438
> * @requires vm.gc=="null"
> * @summary Verify that starting the VM with a small heap works
> - * @run main/othervm -Xmx4m -XX:+UseParallelGC TestSmallHeap
> - * @run main/othervm -Xmx4m -XX:+UseSerialGC TestSmallHeap
> - * @run main/othervm -Xmx4m -XX:+UseG1GC TestSmallHeap
> - * @run main/othervm -Xmx4m -XX:+UseConcMarkSweepGC
> -XX:CMSMarkStackSizeMax=1032 TestSmallHeap
> + * @library /testlibrary /testlibrary/whitebox
> + * @build TestSmallHeap
> + * @run main ClassFileInstaller sun.hotspot.WhiteBox
> + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
> -XX:+WhiteBoxAPI -Xmx2m -XX:+UseParallelGC TestSmallHeap
> + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
> -XX:+WhiteBoxAPI -Xmx2m -XX:+UseSerialGC TestSmallHeap
> + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
> -XX:+WhiteBoxAPI -Xmx2m -XX:+UseG1GC TestSmallHeap
> + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
> -XX:+WhiteBoxAPI -Xmx2m -XX:+UseConcMarkSweepGC
> -XX:CMSMarkStackSizeMax=1032 TestSmallHeap
> *
> * Note: It would be nice to verify the minimal supported heap size here,
> * but that turns out to be quite tricky since we align the heap size
> based
> @@ -47,12 +50,16 @@
>
> import sun.management.ManagementFactoryHelper;
> import static com.oracle.java.testlibrary.Asserts.*;
> +import com.oracle.java.testlibrary.*;
> +import sun.hotspot.WhiteBox;
>
> public class TestSmallHeap {
>
> public static void main(String[] args) {
> + WhiteBox wb = WhiteBox.getWhiteBox();
> + int pageSize = wb.getVMPageSize();
> + long expectedMaxHeap = pageSize * 512;
> String maxHeap =
> ManagementFactoryHelper.getDiagnosticMXBean().getVMOption("MaxHeapSize").getValue();
> - String expectedMaxHeap = "4194304";
> - assertEQ(maxHeap, expectedMaxHeap);
> + assertEQ(Long.parseLong(maxHeap), expectedMaxHeap);
> }
> }
>
>
>> Best regards,
>> Goetz.
>>
>> -----Original Message-----
>> From: Bengt Rutisson [mailto:bengt.rutisson at oracle.com]
>> Sent: Mittwoch, 7. Januar 2015 15:35
>> 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 14:22, Lindenmaier, Goetz wrote:
>>> Hi Bengt,
>>>
>>> there is wb.getVMPageSize().
>>> (see test/runtime/memory/ReadVMPageSize.java)
>> Ah! Good!
>>
>>> But I think the math will be quite complex, I would assume that it depends on
>>> the gc used.
>> I actually think the math is pretty simple and GC independent. One byte
>> in the card table corresponds to 512 bytes of heap. We want at least one
>> page for the card table.
>>
>> So that means that the math is just:
>>
>> minimumHeapSize=pagezise * 512
>>
>> If we set -Xmx2m on the command line we the expectedHeapSize should be
>> the minimum heap size.
>>
>> In the case of 4k page size this will be 2m. In the case of 64k page
>> size it will be 32m.
>>
>>> 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?
>> If you have time it would be great if you can try the WB-version out. If
>> not, I can do it.
>>
>> Thanks,
>> Bengt
>>
>>
>>> 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
More information about the hotspot-gc-dev
mailing list