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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Wed Jan 7 16:04:57 UTC 2015


Hi Bengt,

I had to add the new path to this line:
 * @library /testlibrary /testlibrary/whitebox  /../../test/lib

But then it works fine on the machine with 64K pages.

As you now have everything set up, it would be great if 
you could submit this, as I anyways have to find a sponsor.

You could adapt the comment to the new behavior, though.

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