[Java] Fix out-dated signatures of JNI methods #2756

jovany-wang · 2018-08-28T10:04:00Z

What do these changes do?

When Java code invoke native code by JNI, it will find the function by the function name which is declared in native code(org_ray_spi_impl_DefaultLocalSchedulerClient.cc). If there are some overloading functions, the JNI will find the most matching one by the signature in the comment, like this:

/*
 * Class:     org_ray_spi_impl_DefaultLocalSchedulerClient
 * Method:    _reconstructObjects
 * Signature: (J[BZ)V   /******This is the Signature declaration.******/
 */

Many signatures are not correct yet.
But because there are no overloading, this issue is not triggered.

In this PR, I do these things:

Renamed the native JNI methods and some parameters of JNI methods.
Fixed native JNI methods' signatures by javah tool.
Removed some useless native methods.

Related issue number

N/A

AmplabJenkins · 2018-08-28T11:17:33Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7822/
Test FAILed.

AmplabJenkins · 2018-08-28T12:41:21Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7823/
Test PASSed.

raulchen

Thanks, looks good to me.

Most of the function parameters in org_ray_spi_impl_DefaultLocalSchedulerClient.cc don't have meaningful names. Could you also improve that with this PR? thanks.

robertnishihara

Looks good to me, though we should still get rid of the autogenerated files at some point.

jovany-wang · 2018-08-29T07:55:11Z

I have updated some code.

In conclusion, it contains 3 changes:

Renamed the native JNI methods and some parameters of JNI methods.
Fixed native JNI methods' signatures by javah tool.
Removed some useless native methods.

@robertnishihara @raulchen Could u plz review this again?

jovany-wang · 2018-08-29T08:44:03Z

@eric-jj cc

AmplabJenkins · 2018-08-29T08:57:56Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7852/
Test PASSed.

raulchen

There's lint error in org_ray_spi_impl_DefaultLocalSchedulerClient.cc. Otherwise looks good.

AmplabJenkins · 2018-08-29T12:23:47Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7855/
Test PASSed.

AmplabJenkins · 2018-08-29T12:41:45Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7856/
Test FAILed.

jovany-wang · 2018-08-29T14:03:43Z

I didn't change any py file, and just changed org_ray_spi_impl_DefaultLocalSchedulerClient.h and org_ray_spi_impl_DefaultLocalSchedulerClient.cc which are used for Java.
But in many times, the CI job #1 and #2 can't pass.

=============== 1 failed, 47 passed, 8 skipped in 218.85 seconds ===============
The command "python -m pytest -v test/actor_test.py" exited with 1.

@robertnishihara Do u guys have any idea about it?

robertnishihara · 2018-08-29T20:47:11Z

src/local_scheduler/lib/java/org_ray_spi_impl_DefaultLocalSchedulerClient.cc

@@ -153,7 +153,8 @@ Java_org_ray_spi_impl_DefaultLocalSchedulerClient_nativeReconstructObjects(
  std::vector<ObjectID> object_ids;
  auto len = env->GetArrayLength(objectIds);
  for (int i = 0; i < len; i++) {
-    jbyteArray object_id_bytes = (jbyteArray) env->GetObjectArrayElement(objectIds, i);
+    jbyteArray object_id_bytes =
+        (jbyteArray) env->GetObjectArrayElement(objectIds, i);


We should not use C style casts here

robertnishihara · 2018-08-29T20:47:21Z

src/local_scheduler/lib/java/org_ray_spi_impl_DefaultLocalSchedulerClient.cc

@@ -209,7 +211,8 @@ Java_org_ray_spi_impl_DefaultLocalSchedulerClient_nativeWaitObject(
  std::vector<ObjectID> object_ids;
  auto len = env->GetArrayLength(objectIds);
  for (int i = 0; i < len; i++) {
-    jbyteArray object_id_bytes = (jbyteArray) env->GetObjectArrayElement(objectIds, i);
+    jbyteArray object_id_bytes =
+        (jbyteArray) env->GetObjectArrayElement(objectIds, i);


C style cast

robertnishihara · 2018-08-29T20:48:46Z

@jovan-wong I think those test failures are unrelated. Can you remove the C-style casts? Then I think we can merge it.

raulchen · 2018-08-30T03:22:25Z

@jovan-wong It's weird the first 2 jobs failed consistently for a few times. It should be unrelated. But let's at least make sure that the same test case doesn't fail again. Then I'll merge it.

AmplabJenkins · 2018-08-30T03:22:38Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7879/
Test FAILed.

AmplabJenkins · 2018-08-30T05:34:50Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/7883/
Test PASSed.

jovany-wang requested review from robertnishihara and raulchen August 28, 2018 10:04

raulchen reviewed Aug 28, 2018

View reviewed changes

robertnishihara approved these changes Aug 28, 2018

View reviewed changes

raulchen requested changes Aug 29, 2018

View reviewed changes

raulchen changed the title ~~[Java] Fix methods' signatures for JNI.~~ [Java] Fix out-dated signatures of JNI methods Aug 29, 2018

raulchen approved these changes Aug 29, 2018

View reviewed changes

jovany-wang added 9 commits August 29, 2018 20:58

Fix methods' signatures for JNI.

f0f80d8

Update org_ray_spi_impl_DefaultLocalSchedulerClient.cc

ca28f3e

Rename the parameter name in JNI file.

aac3a83

Clean code.

209f1de

Rename native method.

c6247a3

Remove useless native methods.

fc3b1e1

Add usage comment.

b154a47

Fix linter.

5f0aa4a

Fix linter.

e7ae41a

robertnishihara reviewed Aug 29, 2018

View reviewed changes

Remove C-style casts.

b2b5035

jovany-wang force-pushed the fix_jni branch from e18f29e to b2b5035 Compare August 30, 2018 02:05

Update org_ray_spi_impl_DefaultLocalSchedulerClient.cc

35f31e3

raulchen merged commit 5146334 into ray-project:master Aug 30, 2018

jovany-wang deleted the fix_jni branch August 31, 2018 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Java] Fix out-dated signatures of JNI methods #2756

[Java] Fix out-dated signatures of JNI methods #2756

jovany-wang commented Aug 28, 2018 •

edited

Loading

AmplabJenkins commented Aug 28, 2018

AmplabJenkins commented Aug 28, 2018

raulchen left a comment

robertnishihara left a comment

jovany-wang commented Aug 29, 2018

jovany-wang commented Aug 29, 2018

AmplabJenkins commented Aug 29, 2018

raulchen left a comment

AmplabJenkins commented Aug 29, 2018

AmplabJenkins commented Aug 29, 2018

jovany-wang commented Aug 29, 2018

robertnishihara Aug 29, 2018

robertnishihara Aug 29, 2018

robertnishihara commented Aug 29, 2018

raulchen commented Aug 30, 2018

AmplabJenkins commented Aug 30, 2018

AmplabJenkins commented Aug 30, 2018

[Java] Fix out-dated signatures of JNI methods #2756

[Java] Fix out-dated signatures of JNI methods #2756

Conversation

jovany-wang commented Aug 28, 2018 • edited Loading

What do these changes do?

Related issue number

AmplabJenkins commented Aug 28, 2018

AmplabJenkins commented Aug 28, 2018

raulchen left a comment

Choose a reason for hiding this comment

robertnishihara left a comment

Choose a reason for hiding this comment

jovany-wang commented Aug 29, 2018

jovany-wang commented Aug 29, 2018

AmplabJenkins commented Aug 29, 2018

raulchen left a comment

Choose a reason for hiding this comment

AmplabJenkins commented Aug 29, 2018

AmplabJenkins commented Aug 29, 2018

jovany-wang commented Aug 29, 2018

robertnishihara Aug 29, 2018

Choose a reason for hiding this comment

robertnishihara Aug 29, 2018

Choose a reason for hiding this comment

robertnishihara commented Aug 29, 2018

raulchen commented Aug 30, 2018

AmplabJenkins commented Aug 30, 2018

AmplabJenkins commented Aug 30, 2018

jovany-wang commented Aug 28, 2018 •

edited

Loading