Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java JNI (JPype) Wrapper #2374

Merged
merged 39 commits into from
Sep 21, 2020
Merged

Conversation

adriangonz
Copy link
Contributor

@adriangonz adriangonz commented Sep 3, 2020

What this PR does / why we need it:
Add an experimental option to wrap Java models which leverages the Python inference server under the hood. The communication between the Python and Java side is managed through JNI using JPype.

To make the interop more efficient, this PR also introduces a method to disable the payload serialisation / deserialisation on the Python side (so that is managed by Java). This is controlled through the PAYLOAD_PASSTHROUGH env variable.

To Do:

Which issue(s) this PR fixes:

Work towards #1344

Special notes for your reviewer:
This works is considered as an incubating project.

Does this PR introduce a user-facing change?:

Added incubating approach to wrap Java models, leveraging the Python inference server under the hood.

@seldondev
Copy link
Collaborator

Thu Sep 3 13:27:26 UTC 2020
The logs for [lint] [2] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/2.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=2

@seldondev
Copy link
Collaborator

Thu Sep 3 13:27:32 UTC 2020
The logs for [pr-build] [1] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/1.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=1

@adriangonz adriangonz changed the title Java JNI (JPype) Wrapper WIP : Java JNI (JPype) Wrapper Sep 3, 2020
@adriangonz
Copy link
Contributor Author

While this is still a WIP PR, it would be great to hear your thoughts about the changes introduced in this PR @cliveseldon @axsaucedo @RafalSkolasinski .

Copy link
Contributor

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great @adriangonz ! I added a couple of comments, would be great to hear your thoughts

python/seldon_core/wrapper.py Show resolved Hide resolved
incubating/wrappers/s2i/java-jni/version.txt Outdated Show resolved Hide resolved
doc/source/java-jni/README.md Show resolved Hide resolved
@adriangonz adriangonz changed the title WIP : Java JNI (JPype) Wrapper Java JNI (JPype) Wrapper Sep 18, 2020
@seldondev
Copy link
Collaborator

Fri Sep 18 17:10:16 UTC 2020
The logs for [pr-build] [9] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/9.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=9

@seldondev
Copy link
Collaborator

Fri Sep 18 17:10:23 UTC 2020
The logs for [lint] [10] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/10.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=10

@adriangonz
Copy link
Contributor Author

adriangonz commented Sep 18, 2020

/cc @RafalSkolasinski @axsaucedo

I just added some tests to this PR using the new JNI wrapper. I tried to also add some for the pure Java wrapper but there was some issue there. This can be tackled on a separate PR though.

This should now be ready for review 👍

Copy link
Contributor

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis is great! Nice one @adriangonz - should be good to go once integration passes (excluding known tests with issues)
/approve

incubating/wrappers/s2i/java-jni/java-jni/JavaJNIServer.py Outdated Show resolved Hide resolved
@seldondev seldondev removed the lgtm label Sep 21, 2020
@seldondev
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: axsaucedo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev
Copy link
Collaborator

Mon Sep 21 07:53:08 UTC 2020
The logs for [pr-build] [11] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/11.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=11

@seldondev
Copy link
Collaborator

Mon Sep 21 07:53:17 UTC 2020
The logs for [lint] [12] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2374/12.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2374 --build=12

@seldondev seldondev merged commit b59d2a3 into SeldonIO:master Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants