Add constant dynamic unit tests #2870

r30shah · 2018-09-14T12:51:32Z

Add Java 11 tests for Constant Dynamic mainly involves String object, NULL object and Primitive data types objects. Variation covers running this test in interpreted mode as well as specific JIT compilation modes which checks out JIT's handling of resolved and unresolved Constant Dynamic cases.

Signed-off-by: Rahil Shah rahil@ca.ibm.com

r30shah · 2018-09-14T12:57:50Z

@yanluo7 @fjeremic These are the tests for Constant Dynamic which includes Testing following primitive data types

Int
Float
Double
Long
It also tests Null and String Objects.
@smlambert Can I get a review from Testing team as well? Especially for second commit
ASM generator that we usually got is from release branch which did not have support for Constant Dynamic. So I changed to to get from Snapshots which I am not sure is fine or not.

DanHeidinga

Thanks for writing these tests!

DanHeidinga · 2018-09-14T12:58:58Z

test/functional/Java11andUp/playlist.xml

+	<test>
+		<testCaseName>CondyPrimitive</testCaseName>
+		<variations>
+			<variation>'-Xint'</variation>


I don't think we need an -Xint variation as it tends to be slow to run. A no options variant will typically result in the interpreter executing the test methods which should be sufficient.

Changed to NoOptions in 2d8f421

DanHeidinga · 2018-09-14T12:59:57Z

test/functional/Java11andUp/playlist.xml

+		<variations>
+			<variation>'-Xint'</variation>
+			<variation>'-Xjit:count=1,disableAsyncCompilation'</variation>
+			<variation>'-Xjit:count=0,disableAsyncCompilation'</variation>


Is this to force runtime resolves? Should it also include the rtResolve options to force the jit to always take the resolve path?

Updated to -Xjit:count=1,disableAsyncCompilation,rtResolve in 2d8f421

test/functional/Java11andUp/playlist.xml

DanHeidinga · 2018-09-14T13:44:00Z

test/functional/Java11andUp/src/org/openj9/test/condy/BootstrapMethods.java

+
+public static int bootstrap_constant_int(MethodHandles.Lookup l, String name, Class<?> c, int v) {
+	return v;
+	}


Formatting - the methods should be indented like:

public class BootstrapMethods { public static Object bootstrap_constant_string(MethodHandles.Lookup l, String name, Class<?> c, String s) { return s; } }

test/functional/Java11andUp/src/org/openj9/test/condy/PrimitiveCondyTest.java

fjeremic · 2018-09-14T14:34:42Z

test/TestConfig/scripts/tools/getDependencies.pl

@@ -66,9 +66,9 @@
 # Define a a hash for each dependent jar
 # Contents in the hash should be: url => , fname =>, sha1 =>
 my %asm_all = (
-	url => 'http://central.maven.org/maven2/org/ow2/asm/asm-all/6.0_BETA/asm-all-6.0_BETA.jar',
+	url => 'https://repository.ow2.org/nexus/content/repositories/snapshots/org/ow2/asm/asm/7.0-beta-SNAPSHOT/asm-7.0-beta-20180911.182645-11.jar',


@smlambert @llxia we had to upgrade ASM to a newer level for the condy bytecode support. I'm just verifying this change is ok from the test perspective. Please let us know. Thanks!

Thanks @fjeremic - didn't know you were picking this up, and @tajila and I were just discussing adding this yesterday. It is what is required, and thanks for adding it. I will presume that PR builds should verify that this necessary upgrade does not affect the other tests that rely on asm.

LGTM.

https://ci.eclipse.org/openj9/job/test.getDependency/ - may need to manually launch after merge. It is scheduled to run every hour. If any test build is launched between the merge and new test.getDependency build, the test build will fail.

Same for https://ci.adoptopenjdk.net/view/all/job/test.getDependency/ and internal Jenkins

test/functional/Java11andUp/src/org/openj9/test/condy/PrimitiveCondyTest.java

fjeremic · 2018-09-14T18:41:15Z

I'll wait until @DanHeidinga gives an approval before launching builds for this just in case you may have additional comments on the change.

DanHeidinga

lgtm

r30shah · 2018-09-14T19:00:53Z

Let me squash the commits. I will keep grabbing latest ASM change in separate Commit

fjeremic · 2018-09-14T19:03:34Z

Jenkins test sanity xlinux,plinux,zlinux JDK8,JDK11

r30shah · 2018-09-14T20:12:11Z

It seems like we need asm-all (Failures corresponds to misssing components like asm/tree/*
Which are present in released version of asm-all.jar which seems to combine all components of asm in single jar.
Trying to find out similar asm-all.jar for 7.0 snapshots.

Add Java 11 tests for Constant Dynamic mainly involves String object, NULL object and Primitive data types objects. Variation covers running this test in interpreted mode as well as specific JIT compilation modes which checks out JIT's handling of resolved and unresolved Constant Dynamic cases. Signed-off-by: Rahil Shah <rahil@ca.ibm.com>

For released asm version we get asm-all.jar which combines all asm components in single jar. To test ConstantDynamic helper methods are only available in 7.0 version of asm which is not released yet. This commit grabs asm-7.0 as well which contains necessary classes to generate tests for Constant Dynamic Signed-off-by: Rahil Shah <rahil@ca.ibm.com>

r30shah · 2018-09-14T20:51:02Z

@fjeremic @llxia I made a change, specifically with getting asm commit. Now we will have two asm jars,
One asm-all.jar required by all tests and another asm-7.0.jar needed for Java 11 tests. This is temporary change , as expecting to have all combined jar available for 7.0 version once released. Made corresponding changes Java11andUp/build.xml as well.

fjeremic · 2018-09-14T21:19:48Z

@fjeremic @llxia I made a change, specifically with getting asm commit. Now we will have two asm jars,
One asm-all.jar required by all tests and another asm-7.0.jar needed for Java 11 tests. This is temporary change , as expecting to have all combined jar available for 7.0 version once released. Made corresponding changes Java11andUp/build.xml as well.

@llxia I'm not sure how the test team is tracking all the work but can you open up an Issue somewhere to remember to clean this up once 7.0 is released?

fjeremic · 2018-09-14T21:20:02Z

Jenkins test sanity xlinux,plinux,zlinux JDK8,JDK11

fjeremic · 2018-09-15T02:17:51Z

JDK11 failures due to #2708. This was way before we merged any of the JIT changes in this area so it cannot be related to this change.

DanHeidinga reviewed Sep 14, 2018

View reviewed changes

fjeremic added the comp:test label Sep 14, 2018

fjeremic reviewed Sep 14, 2018

View reviewed changes

fjeremic changed the title ~~WIP: Condy test~~ WIP: Add constant dynamic unit tests Sep 14, 2018

r30shah force-pushed the condyTest branch from 3e1c151 to 2d8f421 Compare September 14, 2018 17:58

r30shah commented Sep 14, 2018

View reviewed changes

test/functional/Java11andUp/src/org/openj9/test/condy/PrimitiveCondyTest.java Show resolved Hide resolved

fjeremic self-assigned this Sep 14, 2018

fjeremic approved these changes Sep 14, 2018

View reviewed changes

fjeremic mentioned this pull request Sep 14, 2018

ConstantDynamic: Add tests for ConstantDynamic #1274

Open

DanHeidinga approved these changes Sep 14, 2018

View reviewed changes

r30shah force-pushed the condyTest branch from 2d8f421 to 76c8a44 Compare September 14, 2018 19:02

fjeremic approved these changes Sep 14, 2018

View reviewed changes

r30shah changed the title ~~WIP: Add constant dynamic unit tests~~ Add constant dynamic unit tests Sep 14, 2018

r30shah added 2 commits September 14, 2018 16:37

r30shah force-pushed the condyTest branch from 76c8a44 to 71ca4c1 Compare September 14, 2018 20:47

llxia approved these changes Sep 14, 2018

View reviewed changes

fjeremic merged commit c26bbd5 into eclipse-openj9:master Sep 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add constant dynamic unit tests #2870

Add constant dynamic unit tests #2870

r30shah commented Sep 14, 2018

r30shah commented Sep 14, 2018

DanHeidinga left a comment

DanHeidinga Sep 14, 2018

r30shah Sep 14, 2018

DanHeidinga Sep 14, 2018

r30shah Sep 14, 2018

DanHeidinga Sep 14, 2018

fjeremic Sep 14, 2018

smlambert Sep 14, 2018

llxia Sep 14, 2018 •

edited

fjeremic commented Sep 14, 2018

DanHeidinga left a comment

r30shah commented Sep 14, 2018 •

edited

fjeremic commented Sep 14, 2018

r30shah commented Sep 14, 2018

r30shah commented Sep 14, 2018

fjeremic commented Sep 14, 2018

fjeremic commented Sep 14, 2018

fjeremic commented Sep 15, 2018

Add constant dynamic unit tests #2870

Add constant dynamic unit tests #2870

Conversation

r30shah commented Sep 14, 2018

r30shah commented Sep 14, 2018

DanHeidinga left a comment

Choose a reason for hiding this comment

DanHeidinga Sep 14, 2018

Choose a reason for hiding this comment

r30shah Sep 14, 2018

Choose a reason for hiding this comment

DanHeidinga Sep 14, 2018

Choose a reason for hiding this comment

r30shah Sep 14, 2018

Choose a reason for hiding this comment

DanHeidinga Sep 14, 2018

Choose a reason for hiding this comment

fjeremic Sep 14, 2018

Choose a reason for hiding this comment

smlambert Sep 14, 2018

Choose a reason for hiding this comment

llxia Sep 14, 2018 • edited

Choose a reason for hiding this comment

fjeremic commented Sep 14, 2018

DanHeidinga left a comment

Choose a reason for hiding this comment

r30shah commented Sep 14, 2018 • edited

fjeremic commented Sep 14, 2018

r30shah commented Sep 14, 2018

r30shah commented Sep 14, 2018

fjeremic commented Sep 14, 2018

fjeremic commented Sep 14, 2018

fjeremic commented Sep 15, 2018

llxia Sep 14, 2018 •

edited

r30shah commented Sep 14, 2018 •

edited