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

Build/distribute framework library and upgrade to TF 2.2.0 #44

Merged
merged 9 commits into from May 18, 2020

Conversation

karllessard
Copy link
Collaborator

@karllessard karllessard commented Apr 28, 2020

This is to fix this issue when loading custom ops built on top of libtensorflow_framework

It builds the TF framework library and attach it to do the native artifacts we distribute. For example:

jar tvf target/tensorflow-core-api-0.1.0-SNAPSHOT-macosx-x86_64.jar 
    99 Tue Apr 28 00:32:46 EDT 2020 META-INF/MANIFEST.MF
     0 Tue Apr 28 00:32:46 EDT 2020 META-INF/
     0 Tue Apr 28 00:32:46 EDT 2020 org/
     0 Tue Apr 28 00:32:46 EDT 2020 org/tensorflow/
     0 Tue Apr 28 00:32:46 EDT 2020 org/tensorflow/internal/
     0 Tue Apr 28 00:32:46 EDT 2020 org/tensorflow/internal/c_api/
     0 Tue Apr 28 00:32:40 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/
     0 Tue Apr 28 00:32:46 EDT 2020 META-INF/maven/
     0 Tue Apr 28 00:32:46 EDT 2020 META-INF/maven/org.tensorflow/
     0 Tue Apr 28 00:32:46 EDT 2020 META-INF/maven/org.tensorflow/tensorflow-core-api/
 11419 Tue Apr 28 00:32:40 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/LICENSE
494616 Tue Apr 28 00:32:38 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/libjnitensorflow.dylib
27689440 Tue Apr 28 00:32:38 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/libtensorflow_framework.2.dylib
255688376 Tue Apr 28 00:32:40 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/libtensorflow.2.dylib
469957 Tue Apr 28 00:32:40 EDT 2020 org/tensorflow/internal/c_api/macosx-x86_64/THIRD_PARTY_TF_JNI_LICENSES
 14489 Wed Apr 22 23:50:18 EDT 2020 META-INF/maven/org.tensorflow/tensorflow-core-api/pom.xml
   108 Thu Apr 23 01:50:26 EDT 2020 META-INF/maven/org.tensorflow/tensorflow-core-api/pom.properties

Unfortunately, that increases each native artifact by 28M but I don't think there is any way to avoid that. Also, I've only build on OSx for now, @saudet can you please validate if the build.sh script is doing the right thing for other platforms as well, especially for Windows?

I'll try to load tensorflow-text extracted from TF2.x packages in the JVM after that.

@karllessard
Copy link
Collaborator Author

@saudet can you please take a look at this again? I'm not sure that's the right way to do it but it seems to work, at least on Linux. Will try on MacOS as well.

@karllessard
Copy link
Collaborator Author

Note that for the op generator, I now run the generator in the build.sh script instead of bazel and pass the TF library path to the command line. This way, we could now use the same binary to generate op wrappers of other libraries than TF (e.g. tensorflow-text or any user custom lib...).

I did the same thing for the java_api_import binary, where it makes a little bit less sense in this context, but again this is just a low-priority internal utility and I don't want to complexify things just for it.

@karllessard karllessard changed the title Build and distribute framework library Build/distribute framework library and upgrade to TF 2.2.0 May 13, 2020
@karllessard karllessard marked this pull request as ready for review May 13, 2020 03:20
@saudet
Copy link
Contributor

saudet commented May 13, 2020

Looks alright at first glance, although libcustom_op_test.so won't work on Windows for sure, but before going into more details, why is there such a big difference between 2.2.0-rc3 and 2.2.0 in the files generated?

@karllessard
Copy link
Collaborator Author

karllessard commented May 13, 2020

There is a bunch of Xla* ops that appeared from the last op generation and I'm not too sure why because these ops seem older than 2.2.0. But anyhow, I thought they were a great addition so I kept them.

For the *.so library, in fact it is a fake library name because Bazel cannot add symlinks to a cc_library, you need to create a cc_binary, see this novel about this. So the hack might work on Windows as well, @zaleslaw would you mind give it a try on your machine and build this branch?

@karllessard
Copy link
Collaborator Author

I'll wait for PR #54 CI build to complete, to see if it is successful, then I'll rebase and check how it goes with this one before merging it.

@karllessard karllessard force-pushed the add-framework-lib branch 3 times, most recently from aed619d to 49b9696 Compare May 15, 2020 23:20
@karllessard karllessard force-pushed the add-framework-lib branch 3 times, most recently from ddba0d3 to 4b52381 Compare May 17, 2020 14:13
@karllessard karllessard merged commit eeb6778 into tensorflow:master May 18, 2020
@karllessard karllessard deleted the add-framework-lib branch May 18, 2020 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TF 2.x Java Binding @rpath/libtensorflow_framework.1.dylib
2 participants