RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

Kharbas, Kishor

Hi!

 

I have developed a test plan for the implementation of 8171181.

I would appreciate a review and further guidance from the gc-dev members. I am hoping to get everything done well before 18.3 code freeze (have a vacation planned during that time).

 

Test plan: https://bugs.openjdk.java.net/browse/JDK-8190828

Test webrev: http://cr.openjdk.java.net/~kkharbas/8190980/webrev.01/

JEP: https://bugs.openjdk.java.net/browse/JDK-8171181

Implementation webrev : http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15/

 

Thank you!

Kishor

Reply | Threaded
Open this post in threaded view
|

Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

sangheon.kim@oracle.com
Hi Kishor,

On 11/13/2017 03:51 PM, Kharbas, Kishor wrote:

Hi!

 

I have developed a test plan for the implementation of 8171181.

I would appreciate a review and further guidance from the gc-dev members. I am hoping to get everything done well before 18.3 code freeze (have a vacation planned during that time).

 

Test plan: https://bugs.openjdk.java.net/browse/JDK-8190828

Test webrev: http://cr.openjdk.java.net/~kkharbas/8190980/webrev.01/

Looking at the comment at 8190980, webrev.2 seems the latest one, so my comments are for the webrev.2.

--------------------------------------
test/hotspot/jtreg/gc/TestAllocateHeapAtMultiple.java

  51     String[] extraOptsList = new String[] {
  52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
  53       "-Xmx32m -Xms32m -XX:-UseCompressedOops",     // 2. With compressedoops disabled.
  54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
  55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
  56       "-Xmx4g -Xms4g -XX:+UseLargePages",           // 5. Set UseLargePages.
  57       "-Xmx4g -Xms4g -XX:+UseNUMA"                  // 6. Set UseNUMA.
  58     };
- I think we do differently to run sub-tests. Maybe SQE folks would give better comment on this.
e.g. TestAllocationInEden.java
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 9 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 47 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions

52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
- I think these 3 sub-tests are testing different compressed oop modes. I would recommend to include other 1 type(non-zero based) as well. In addition, adding the comment also would help increase the readability.

--------------------------------------
test/hotspot/jtreg/gc/stress/gcbasher/TestGCBasherWithAllocateHeapAt.java

1) This seems identical to TestGCBasherWithG1.java, how about just adding another '@run'?
i.e.  adding "* @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC  -XX:AllocateHeapAt=. TestGCBasherWithG1 120000"

2) Don't we need testing for other GC types as well? i.e. Serial, Parallel and CMS.

2  * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
- Is this intended to start from 2016 as this seems to be copied from TestGCBasherWithXXX.java?

34  * @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC -XX:AllocateHeapAt=. TestGCBasherWithAllocateHeapAt 120000
- Are there any reason to use timeout of 500? TestGCBasherWithG1 is using 200ms.

Thanks,
Sangheon




Reply | Threaded
Open this post in threaded view
|

RE: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

Kharbas, Kishor

Hi Sangheon!

Thanks for the reply. Please find my reply inline.

 

From: sangheon.kim [mailto:[hidden email]]
Sent: Thursday, November 16, 2017 10:28 PM
To: Kharbas, Kishor <[hidden email]>; [hidden email]
Subject: Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

 

Hi Kishor,

On 11/13/2017 03:51 PM, Kharbas, Kishor wrote:

Hi!

 

I have developed a test plan for the implementation of 8171181.

I would appreciate a review and further guidance from the gc-dev members. I am hoping to get everything done well before 18.3 code freeze (have a vacation planned during that time).

 

Test plan: https://bugs.openjdk.java.net/browse/JDK-8190828

Test webrev: http://cr.openjdk.java.net/~kkharbas/8190980/webrev.01/

Looking at the comment at 8190980, webrev.2 seems the latest one, so my comments are for the webrev.2.

--------------------------------------
test/hotspot/jtreg/gc/TestAllocateHeapAtMultiple.java

  51     String[] extraOptsList = new String[] {
  52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
  53       "-Xmx32m -Xms32m -XX:-UseCompressedOops",     // 2. With compressedoops disabled.
  54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
  55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
  56       "-Xmx4g -Xms4g -XX:+UseLargePages",           // 5. Set UseLargePages.
  57       "-Xmx4g -Xms4g -XX:+UseNUMA"                  // 6. Set UseNUMA.
  58     };
- I think we do differently to run sub-tests. Maybe SQE folks would give better comment on this.
e.g. TestAllocationInEden.java
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 9 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 47 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions

[Kharbas, Kishor] In my tests I use stdout to check for log generated by –Xlog:heap+gc=info to test correct allocation of Heap. That’s why I need to spawn a process from within this test to get a handle on stdout, which (as per my limited knowledge) is not possible if I use @run. Please correct me if I am wrong.


52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
- I think these 3 sub-tests are testing different compressed oop modes. I would recommend to include other 1 type(non-zero based) as well. In addition, adding the comment also would help increase the readability.

[Kharbas, Kishor] Yes I can do that. I need to specify heap size close to 32 GB right? There should be that much disk space available in the test environment, do you think that would be problem?


--------------------------------------
test/hotspot/jtreg/gc/stress/gcbasher/TestGCBasherWithAllocateHeapAt.java

1) This seems identical to TestGCBasherWithG1.java, how about just adding another '@run'?
i.e.  adding "* @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC  -XX:AllocateHeapAt=. TestGCBasherWithG1 120000"

2) Don't we need testing for other GC types as well? i.e. Serial, Parallel and CMS.

[Kharbas, Kishor] The idea was to exercise the Java heap (allocated on a file) by some stress test. I thought one GC option would suffice, do you think we need more?


2  * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
- Is this intended to start from 2016 as this seems to be copied from TestGCBasherWithXXX.java?

[Kharbas, Kishor] I will change this.


34  * @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC -XX:AllocateHeapAt=. TestGCBasherWithAllocateHeapAt 120000
- Are there any reason to use timeout of 500? TestGCBasherWithG1 is using 200ms.

[Kharbas, Kishor] I increased the timeout since heap is mapped to a file on disk (either HDD or SSD depending on test environment) which makes the test run slower.

Thanks,
Sangheon




JEP: https://bugs.openjdk.java.net/browse/JDK-8171181

Implementation webrev : http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15/

 

Thank you!

Kishor

 

Reply | Threaded
Open this post in threaded view
|

Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

sangheon.kim@oracle.com
Hi Kishor,


On 11/17/2017 02:59 PM, Kharbas, Kishor wrote:

Hi Sangheon!

Thanks for the reply. Please find my reply inline.

 

From: sangheon.kim [[hidden email]]
Sent: Thursday, November 16, 2017 10:28 PM
To: Kharbas, Kishor [hidden email]; [hidden email]
Subject: Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

 

Hi Kishor,

On 11/13/2017 03:51 PM, Kharbas, Kishor wrote:

Hi!

 

I have developed a test plan for the implementation of 8171181.

I would appreciate a review and further guidance from the gc-dev members. I am hoping to get everything done well before 18.3 code freeze (have a vacation planned during that time).

 

Test plan: https://bugs.openjdk.java.net/browse/JDK-8190828

Test webrev: http://cr.openjdk.java.net/~kkharbas/8190980/webrev.01/

Looking at the comment at 8190980, webrev.2 seems the latest one, so my comments are for the webrev.2.

--------------------------------------
test/hotspot/jtreg/gc/TestAllocateHeapAtMultiple.java

  51     String[] extraOptsList = new String[] {
  52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
  53       "-Xmx32m -Xms32m -XX:-UseCompressedOops",     // 2. With compressedoops disabled.
  54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
  55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
  56       "-Xmx4g -Xms4g -XX:+UseLargePages",           // 5. Set UseLargePages.
  57       "-Xmx4g -Xms4g -XX:+UseNUMA"                  // 6. Set UseNUMA.
  58     };
- I think we do differently to run sub-tests. Maybe SQE folks would give better comment on this.
e.g. TestAllocationInEden.java
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 9 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 47 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions

[Kharbas, Kishor] In my tests I use stdout to check for log generated by –Xlog:heap+gc=info to test correct allocation of Heap. That’s why I need to spawn a process from within this test to get a handle on stdout, which (as per my limited knowledge) is not possible if I use @run. Please correct me if I am wrong.

Please keep as is.
I think handling stdout seems okay with @run approach but @run doesn't allow to add more vm options. In this test, we need 'test_dir', so we can't use @run.



52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
- I think these 3 sub-tests are testing different compressed oop modes. I would recommend to include other 1 type(non-zero based) as well. In addition, adding the comment also would help increase the readability.

[Kharbas, Kishor] Yes I can do that. I need to specify heap size close to 32 GB right? There should be that much disk space available in the test environment, do you think that would be problem?

No need so huge heap. Please refer UseComporessedOops.java for an example of each cases.
universe.hpp:378 also has good explanation.
e.g. -Xmx32m -XX:HeapBaseMinAddress=0x800000008 also uses non-zero based.



--------------------------------------
test/hotspot/jtreg/gc/stress/gcbasher/TestGCBasherWithAllocateHeapAt.java

1) This seems identical to TestGCBasherWithG1.java, how about just adding another '@run'?
i.e.  adding "* @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC  -XX:AllocateHeapAt=. TestGCBasherWithG1 120000"

2) Don't we need testing for other GC types as well? i.e. Serial, Parallel and CMS.

[Kharbas, Kishor] The idea was to exercise the Java heap (allocated on a file) by some stress test. I thought one GC option would suffice, do you think we need more?


Let's leave as is.


2  * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
- Is this intended to start from 2016 as this seems to be copied from TestGCBasherWithXXX.java?

[Kharbas, Kishor] I will change this.

OK.
But, if you agree to go 1) option(adding @run, instead of making TestGCBasherWithAllocateHeapAt.java, this comment can be ignored.



34  * @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC -XX:AllocateHeapAt=. TestGCBasherWithAllocateHeapAt 120000
- Are there any reason to use timeout of 500? TestGCBasherWithG1 is using 200ms.

[Kharbas, Kishor] I increased the timeout since heap is mapped to a file on disk (either HDD or SSD depending on test environment) which makes the test run slower.

Make sense to have longer timeout, not sure additional 300ms is good enough though.

At least when I ran your 8190308(webrev.15) with this webrev.2, all tier1~tier5 tests were okay. There were some failures but those are known issues.

Thanks,
Sangheon



Thanks,
Sangheon




JEP: https://bugs.openjdk.java.net/browse/JDK-8171181

Implementation webrev : http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15/

 

Thank you!

Kishor

 


Reply | Threaded
Open this post in threaded view
|

RE: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

Kharbas, Kishor

Hi Sagheon,

 

I have new webrev at – http://cr.openjdk.java.net/~kkharbas/8190980/webrev.03

and my reply inline.

 

Thank you!

From: sangheon.kim [mailto:[hidden email]]
Sent: Friday, November 17, 2017 3:41 PM
To: Kharbas, Kishor <[hidden email]>; [hidden email]
Subject: Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

 

Hi Kishor,

On 11/17/2017 02:59 PM, Kharbas, Kishor wrote:

Hi Sangheon!

Thanks for the reply. Please find my reply inline.

 

From: sangheon.kim [[hidden email]]
Sent: Thursday, November 16, 2017 10:28 PM
To: Kharbas, Kishor [hidden email]; [hidden email]
Subject: Re: RFR(M): 8190828: Test plan: JEP 8171181: Support heap allocation on alternative memory devices

 

Hi Kishor,

On 11/13/2017 03:51 PM, Kharbas, Kishor wrote:

Hi!

 

I have developed a test plan for the implementation of 8171181.

I would appreciate a review and further guidance from the gc-dev members. I am hoping to get everything done well before 18.3 code freeze (have a vacation planned during that time).

 

Test plan: https://bugs.openjdk.java.net/browse/JDK-8190828

Test webrev: http://cr.openjdk.java.net/~kkharbas/8190980/webrev.01/

Looking at the comment at 8190980, webrev.2 seems the latest one, so my comments are for the webrev.2.

--------------------------------------
test/hotspot/jtreg/gc/TestAllocateHeapAtMultiple.java

  51     String[] extraOptsList = new String[] {
  52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
  53       "-Xmx32m -Xms32m -XX:-UseCompressedOops",     // 2. With compressedoops disabled.
  54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
  55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
  56       "-Xmx4g -Xms4g -XX:+UseLargePages",           // 5. Set UseLargePages.
  57       "-Xmx4g -Xms4g -XX:+UseNUMA"                  // 6. Set UseNUMA.
  58     };
- I think we do differently to run sub-tests. Maybe SQE folks would give better comment on this.
e.g. TestAllocationInEden.java
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 9 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions
...
 *                   TestAllocationInEden 10m 47 EDEN
 * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions


[Kharbas, Kishor] In my tests I use stdout to check for log generated by –Xlog:heap+gc=info to test correct allocation of Heap. That’s why I need to spawn a process from within this test to get a handle on stdout, which (as per my limited knowledge) is not possible if I use @run. Please correct me if I am wrong.

Please keep as is.
I think handling stdout seems okay with @run approach but @run doesn't allow to add more vm options. In this test, we need 'test_dir', so we can't use @run.

[Kharbas, Kishor] Ok. Thanks





52       "-Xmx32m -Xms32m -XX:+UseCompressedOops",     // 1. With compressedoops enabled.
54       "-Xmx32m -Xms32m -XX:HeapBaseMinAddress=3g",  // 3. With user specified HeapBaseMinAddress.
55       "-Xmx4g -Xms4g",                              // 4. With larger heap size (UnscaledNarrowOop not possible).
- I think these 3 sub-tests are testing different compressed oop modes. I would recommend to include other 1 type(non-zero based) as well. In addition, adding the comment also would help increase the readability.


[Kharbas, Kishor] Yes I can do that. I need to specify heap size close to 32 GB right? There should be that much disk space available in the test environment, do you think that would be problem?

No need so huge heap. Please refer UseComporessedOops.java for an example of each cases.
universe.hpp:378 also has good explanation.
e.g. -Xmx32m -XX:HeapBaseMinAddress=0x800000008 also uses non-zero based.

[Kharbas, Kishor] I added this sub-test, used the options from UseCompressedOops.java for same non-zero base, unscaled case.


--------------------------------------
test/hotspot/jtreg/gc/stress/gcbasher/TestGCBasherWithAllocateHeapAt.java

1) This seems identical to TestGCBasherWithG1.java, how about just adding another '@run'?
i.e.  adding "* @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC  -XX:AllocateHeapAt=. TestGCBasherWithG1 120000"

2) Don't we need testing for other GC types as well? i.e. Serial, Parallel and CMS.


[Kharbas, Kishor] The idea was to exercise the Java heap (allocated on a file) by some stress test. I thought one GC option would suffice, do you think we need more?

 

Let's leave as is.

[Kharbas, Kishor] Ok Thanks



2  * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
- Is this intended to start from 2016 as this seems to be copied from TestGCBasherWithXXX.java?


[Kharbas, Kishor] I will change this.

OK.
But, if you agree to go 1) option(adding @run, instead of making TestGCBasherWithAllocateHeapAt.java, this comment can be ignored.

[Kharbas, Kishor] I feel it’s better to keep this separate so as to not give an impression that AllocateHeapAt is tied to G1GC. Change the copyright.

 


34  * @run main/othervm/timeout=500 -Xlog:gc*=info -Xmx256m -server -XX:+UseG1GC -XX:AllocateHeapAt=. TestGCBasherWithAllocateHeapAt 120000
- Are there any reason to use timeout of 500? TestGCBasherWithG1 is using 200ms.

[Kharbas, Kishor] I increased the timeout since heap is mapped to a file on disk (either HDD or SSD depending on test environment) which makes the test run slower.

Make sense to have longer timeout, not sure additional 300ms is good enough though.

[Kharbas, Kishor] I my environment even 200s did not fail, but I observed it taking longer. Whole or part of file is paged in memory so run time is not very bad.


At least when I ran your 8190308(webrev.15) with this webrev.2, all tier1~tier5 tests were okay. There were some failures but those are known issues.

Thanks,
Sangheon




Thanks,
Sangheon





JEP: https://bugs.openjdk.java.net/browse/JDK-8171181

Implementation webrev : http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15/

 

Thank you!

Kishor