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

Create no GPU build for Tritonserver #1151

Merged
merged 17 commits into from
Feb 23, 2022
Merged

Create no GPU build for Tritonserver #1151

merged 17 commits into from
Feb 23, 2022

Conversation

jbkyang-nvi
Copy link
Contributor

As the current common use case is for CPU-only builds, change javacpp-presets dependencies to reflect that

@saudet
Copy link
Member

saudet commented Feb 18, 2022

Looks good, but how different is this from pull #1146?

@jbkyang-nvi
Copy link
Contributor Author

Looks good, but how different is this from pull #1146?

This adds a build section that's specifically for building the .jar library. However, the two PRs are basically the same. Since moving the presets to Tritonserver will take a bigger effort, I see this as a intermediate step. 😅

@saudet
Copy link
Member

saudet commented Feb 18, 2022

Ok, I see. Now, build/pom.xml and samples/pom.xml are essentially the same. I think it would make more sense to add the lines for maven-shade-plugin in platform/pom.xml, so it gets created as part of the normal build, if that's the intention, and then name it something like tritonserver-platform-shaded.jar to differentiate it from "normal" platform artifacts.

BTW, currently there are also tritonserver-...-redist artifacts that bundle the library files of Triton itself. This split is done for large libraries like CUDA, MKL, and TensorRT so that users may have the option of using libraries already found on their systems instead of downloading huge binaries, but Triton itself is only 5~6 MB. Would it be OK for the size of the JAR file to be 6 MB? That would reduce the number of choices to make and the number of JAR files users have to deal with. Conversely, if we always want to make sure users have Triton installed on their system (or their containers), we should never bundle Triton itself and get rid of those "-redist" artifacts. What do you think? /cc @jackyh

@jbkyang-nvi
Copy link
Contributor Author

Ok, I see. Now, build/pom.xml and samples/pom.xml are essentially the same. I think it would make more sense to add the lines for maven-shade-plugin in platform/pom.xml, so it gets created as part of the normal build, if that's the intention, and then name it something like tritonserver-platform-shaded.jar to differentiate it from "normal" platform artifacts.

Does this current addition to the platform code make sense?

BTW, currently there are also tritonserver-...-redist artifacts that bundle the library files of Triton itself. This split is done for large libraries like CUDA, MKL, and TensorRT so that users may have the option of using libraries already found on their systems instead of downloading huge binaries, but Triton itself is only 5~6 MB. Would it be OK for the size of the JAR file to be 6 MB? That would reduce the number of choices to make and the number of JAR files users have to deal with. Conversely, if we always want to make sure users have Triton installed on their system (or their containers), we should never bundle Triton itself and get rid of those "-redist" artifacts. What do you think? /cc @jackyh

Triton library is 21MB by itself... does that mean in the redist jar file squeezed everything down to 6MB, including library and bindings? I think we can probably remove the redist folder. Lets see what @jackyh thinks

@saudet
Copy link
Member

saudet commented Feb 18, 2022

Does this current addition to the platform code make sense?

Yes, looking good! Thanks

Triton library is 21MB by itself... does that mean in the redist jar file squeezed everything down to 6MB, including library and bindings? I think we can probably remove the redist folder. Lets see what @jackyh thinks

Yes, it's 21 MB uncompressed, for example, look inside this JAR file:
https://repo1.maven.org/maven2/org/bytedeco/tritonserver/2.18-1.5.7/tritonserver-2.18-1.5.7-linux-x86_64-redist.jar

@saudet
Copy link
Member

saudet commented Feb 21, 2022

I've updated the samples/pom.xml file to use that shaded artifact. This looks good to merge for now, so we can decide later what to do with the redist artifact, if that's OK?

@jbkyang-nvi
Copy link
Contributor Author

I've updated the samples/pom.xml file to use that shaded artifact. This looks good to merge for now, so we can decide later what to do with the redist artifact, if that's OK?

Thanks for making the changes Samuel! Happy to have it merged. We can chat about redist later

@saudet saudet merged commit 4944440 into bytedeco:master Feb 23, 2022
@jbkyang-nvi jbkyang-nvi deleted the kyang-cpu-only branch February 23, 2022 01:46
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.

2 participants