Repository navigation
Refactor JNI code in C++ into Java code with JavaCPP - #18
Conversation
| throw new IndexOutOfBoundsException("MetaGraphDef is too large to serialize into a byte[] array"); | ||
| } else { | ||
| byte[] jmetagraph_def = new byte[(int)metagraph_def.length()]; | ||
| new BytePointer(metagraph_def.data()).get(jmetagraph_def); |
There was a problem hiding this comment.
I suggest we move this array copy stunt in AbstractTF_Buffer as a utility method like toBytes(), since TF_Buffer is meant to be generic.
Alternatively, we could retain a reference to metaGraphDef and return its data as a ByteBuffer to the user instead of an array, as it is only used for deserializing the proto message... But I'm wondering why we just don't import those proto classes in the core API and return typed objects to the user like MetaGraphDef instead of leaving him the burden to serialize/deserialize the proto messages.
What do you think @sjamesr ? Is the original reason of leaving the protos outside the Java client was only to avoid an extra dependency to grpc?
|
@saudet , this PR is still marked as WIP, looks like it's ready to be merged now? |
|
I was thinking we could do all of it together before merging? It shouldn't take me more than a few days to do... |
|
As you wish, we can do it iteratively with smaller PRs as well |
|
Let's talk about it at the meeting tomorrow :) |
dd6373b to
55b7b5b
Compare
karllessard
left a comment
There was a problem hiding this comment.
Looks good @saudet , I just left a few questions and remarks here and there.
I didn't compared yet the old JNI code with the new Java implementations but I think we can also rely on the Junit coverage for this.
This is a WIP to remove the manually written JNI code from tensorflow-core-api. In the first commit for SavedModelBundle.java, I replaced about 100 lines of JNI code in C++ with about 40 lines of code in Java, and the unit tests still pass. We need to do the same with the rest of the JNI code, which shouldn't take me that long to do by myself, but if anyone is interested in helping, please let me know and I will give you write access to my fork!
There is also a lot of code in the Java section of the wrappers that is redundant with JavaCPP and that we will be able to remove in a second phase.
/cc @tzolov