Add bazel build for JNI code #3918

guoyuhong · 2019-01-31T16:02:30Z

What do these changes do?

This PR adds the bazel code to build the Java Raylet Client JNI code. From current cmake process, the output lib name differs in MacOS than in Linux.

Currently, I copy this file to folder $(location ray_pkg)/src/ray/raylet/ corresponding to build/src/ray/raylet of cmake. I think the final path should be determine in #3898 .

Now I can use find bazel-genfiles/ -name *libraylet_library_java.* to find the dynamic lib.

Related issue number

AmplabJenkins · 2019-01-31T18:50:42Z

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

AmplabJenkins · 2019-02-01T11:10:51Z

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

AmplabJenkins · 2019-02-01T11:56:51Z

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

AmplabJenkins · 2019-02-02T12:18:42Z

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

AmplabJenkins · 2019-02-02T13:01:34Z

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

robertnishihara · 2019-02-02T18:56:32Z

BUILD.bazel

-    """,
+        mv python $(location ray_pkg) &&
+        mkdir -p $(location ray_pkg)/src/ray/raylet &&
+        for f in $(locations //:raylet_library_java); do if [[ $$f == *".so"* ]]; then cp $$f $(location ray_pkg)/src/ray/raylet/"""


What is $$?

When put command in """ string, $$variable is like $variable in shell.

raulchen

The code doesn't seem to be well formatted. You can format it with yapf -i as the syntax is compatible with Python.

raulchen · 2019-02-03T06:04:22Z

BUILD.bazel

+            "@bazel_tools//src/conditions:windows": "libraylet_library_java.dll",
+            "@bazel_tools//src/conditions:darwin": "libraylet_library_java.dylib",
+            "//conditions:default": "libraylet_library_java.so"})
+        + "; fi done &&" +


It seems that the default output file is always suffixed by .so, and you want to rename it with different suffixes based on the OS?
It's weird that why it doesn't generate different suffixes in the first place.

I tried to do that at the beginning, but failed... I used this as a workaround.

guoyuhong · 2019-02-03T06:47:37Z

@raulchen I tried yapf -i and there are too many changed lines, which may cause a lot of conflict with @pcmoritz 's PR. I will left it unchanged until the Bazel scripts are ready.

In this PR, I only wanted to add the JNI for raylet client at the beginning. Then, I found there are many missing pieces for enabling Java. I added the JNI for Plasma Client and Arrow Java library and Raylet Java library. For the 2 java libraries, currently, it is very simple building script as a mark to remind us which is missing. We may later consider how to use it correctly. For example, the binary paths in RayConfig.java should be changed accordingly. ray.default.conf’s name should also be changed because Bazel does not allow *.conf file to be included in the jar file. However, we may put that to later PRs.

AmplabJenkins · 2019-02-03T08:52:26Z

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

raulchen · 2019-02-03T08:56:30Z

BUILD.bazel

+    name="ray_java_pkg",
+    srcs=glob([
+        "java/*.java",
+        "java/*.properties",


I tried this, the generated jar file doesn't capture anything.
I think this should be java/**/*.java, right?
Also, we need to add the external dependencies.

A few more comments:

instead of defining single big java library, we should probably define each module (api, runtime, test, etc) as a library.

We should probably follow this tutorial to properly set up the java libraries.

It seems that you want to set up java libraries later. If so, we should probably remove this ray_java_pkg from this PR, because it doesn't actually do anything.

raulchen · 2019-02-03T09:03:15Z

@raulchen I tried yapf -i and there are too many changed lines, which may cause a lot of conflict with @pcmoritz 's PR. I will left it unchanged until the Bazel scripts are ready.

In this PR, I only wanted to add the JNI for raylet client at the beginning. Then, I found there are many missing pieces for enabling Java. I added the JNI for Plasma Client and Arrow Java library and Raylet Java library. For the 2 java libraries, currently, it is very simple building script as a mark to remind us which is missing. We may later consider how to use it correctly. For example, the binary paths in RayConfig.java should be changed accordingly. ray.default.conf’s name should also be changed because Bazel does not allow *.conf file to be included in the jar file. However, we may put that to later PRs.

Regarding the format, I can later open a PR to add linter in CI. But I think we can do that before bazel is completely ready.

Regarding java libraries, I left some comments in line.

guoyuhong · 2019-02-03T12:38:30Z

@pcmoritz I have two questions about this PR.

In bazel/BUILD.plasma, I need to use ../../external/local_jdk/include to include the JNI headers, which is different than the one in BUILD.bazel. Is there a way to unify them?
I cannot specify the cc_library to output .dylib directly. I choose to rename it during copying process. Is there a way to specify it during the building process?

AmplabJenkins · 2019-02-03T14:03:59Z

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

AmplabJenkins · 2019-02-03T16:17:45Z

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

raulchen · 2019-02-04T08:17:19Z

I cannot specify the cc_library to output .dylib directly. I choose to rename it during copying process. Is there a way to specify it during the building process?

@guoyuhong Bazel doesn't seem to support this at this moment. Tensorflow uses a genrule (see https://github.com/tensorflow/tensorflow/blob/53bd138262df6e141c0e196a262f4ae760c2cf5f/tensorflow/java/BUILD#L418-L427) to do renaming.

guoyuhong · 2019-02-04T11:51:21Z

@raulchen Thanks! I have updated the bazel script using the genrule.
I also found that if BUILD.plasma is not at the root dir, we need to add ../../ as a workaround to find jni.h, which is discussed in this page.

AmplabJenkins · 2019-02-04T14:31:03Z

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

pcmoritz

This looks good to me!

guoyuhong requested a review from pcmoritz January 31, 2019 16:02

guoyuhong mentioned this pull request Feb 1, 2019

Make Bazel the default build system #3898

Merged

guoyuhong changed the title ~~Add bazel build for JNI code~~ [WIP] Add bazel build for JNI code Feb 1, 2019

raulchen self-requested a review February 2, 2019 09:52

guoyuhong changed the title ~~[WIP] Add bazel build for JNI code~~ Add bazel build for JNI code Feb 2, 2019

robertnishihara reviewed Feb 2, 2019

View reviewed changes

raulchen reviewed Feb 3, 2019

View reviewed changes

Yuhong Guo added 7 commits February 3, 2019 22:07

Add bazel build for JNI code

3064b5b

clean

93fdcc4

Add plasma client JNI build process

c3a37a5

refine

ac967f8

clean linux part

e423ca3

Add Java Library

2052b3e

Remove java library

d4b1f27

guoyuhong force-pushed the JniBazel branch from dd3cd91 to d4b1f27 Compare February 3, 2019 14:08

Generate dylib after build using genrule

1180842

pcmoritz approved these changes Feb 4, 2019

View reviewed changes

pcmoritz merged commit add8ae7 into ray-project:master Feb 4, 2019

guoyuhong mentioned this pull request Feb 8, 2019

Lint Bazel scripts #3986

Closed

guoyuhong deleted the JniBazel branch February 13, 2019 10:23

robertnishihara mentioned this pull request Mar 5, 2019

Build Ray Java code with Bazel. #4273

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bazel build for JNI code #3918

Add bazel build for JNI code #3918

guoyuhong commented Jan 31, 2019

AmplabJenkins commented Jan 31, 2019

AmplabJenkins commented Feb 1, 2019

AmplabJenkins commented Feb 1, 2019

AmplabJenkins commented Feb 2, 2019

AmplabJenkins commented Feb 2, 2019

robertnishihara Feb 2, 2019

guoyuhong Feb 3, 2019

raulchen left a comment

raulchen Feb 3, 2019

guoyuhong Feb 3, 2019

guoyuhong commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

raulchen Feb 3, 2019

raulchen commented Feb 3, 2019

guoyuhong commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

raulchen commented Feb 4, 2019

guoyuhong commented Feb 4, 2019

AmplabJenkins commented Feb 4, 2019

pcmoritz left a comment

Add bazel build for JNI code #3918

Add bazel build for JNI code #3918

Conversation

guoyuhong commented Jan 31, 2019

What do these changes do?

Related issue number

AmplabJenkins commented Jan 31, 2019

AmplabJenkins commented Feb 1, 2019

AmplabJenkins commented Feb 1, 2019

AmplabJenkins commented Feb 2, 2019

AmplabJenkins commented Feb 2, 2019

robertnishihara Feb 2, 2019

Choose a reason for hiding this comment

guoyuhong Feb 3, 2019

Choose a reason for hiding this comment

raulchen left a comment

Choose a reason for hiding this comment

raulchen Feb 3, 2019

Choose a reason for hiding this comment

guoyuhong Feb 3, 2019

Choose a reason for hiding this comment

guoyuhong commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

raulchen Feb 3, 2019

Choose a reason for hiding this comment

raulchen commented Feb 3, 2019

guoyuhong commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

AmplabJenkins commented Feb 3, 2019

raulchen commented Feb 4, 2019

guoyuhong commented Feb 4, 2019

AmplabJenkins commented Feb 4, 2019

pcmoritz left a comment

Choose a reason for hiding this comment