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.

Sign up for GitHub

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

Jump to bottom

[REVIEW] Remove unused native deps from java library [skip-ci] #5298

Merged
merged 6 commits into from
May 27, 2020
Merged

[REVIEW] Remove unused native deps from java library [skip-ci] #5298

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eebed57
Step one remove unused libraries
revans2 May 27, 2020
2f41c1f
Cleanup java dependencies
revans2 May 27, 2020
c7065dd
Update changelog
revans2 May 27, 2020
0f47cc6
Addressed review comments
revans2 May 27, 2020
1bc72f5
spell check
revans2 May 27, 2020
7d7904c
Merge branch 'branch-0.14' into fix-deps
revans2 May 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
- PR #5213 Documentation enhancements to `cudf` python APIs
- PR #5251 Fix more mispellings in cpp comments and strings
- PR #5270 Add support to check for "NaT" and "None" strings while typecasting to `datetime64`
- PR #5298 Remove unused native deps from java library

## Bug Fixes

Expand Down
68 changes: 50 additions & 18 deletions java/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,45 @@
</plugins>
</build>
</profile>
<profile>
<id>COPY_NVRTC</id>
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why we need this change for this PR, or is this change also about static linking and marking dependencies as required and optional? My understanding of static linking is limited so pardon any naivety.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes so we can statically link against cudart, but nvrtc which is the runtime compiler we cannot statically link against and it is tied to the cuda release version 10.02, 10.1, etc. This copies it into the jar when we are doing static linking.

<activation>
<property>
<name>CUDA_STATIC_RUNTIME</name>
<value>ON</value>
</property>
</activation>
<build>
<plugins>
<plugin>
<artifactId>maven-resources-plugin</artifactId>
<executions>
<execution>
<id>copy-nvrtc</id>
<phase>validate</phase>
<goals>
<goal>copy-resources</goal>
</goals>
<configuration>
<overwrite>true</overwrite>
<skip>${skipNativeCopy}</skip>
<outputDirectory>${basedir}/target/native-deps/${os.arch}/${os.name}</outputDirectory>
<resources>
<resource>
<!--Set by groovy script-->
<directory>${cuda.path}</directory>
<includes>
<include>libnvrtc.so</include>
jlowe marked this conversation as resolved.
Show resolved Hide resolved
</includes>
</resource>
</resources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>

<build>
Expand Down Expand Up @@ -413,6 +452,17 @@
} else {
pom.properties['cuda.classifier'] = ''
}

if (pom.properties['CUDA_STATIC_RUNTIME'] == 'ON') {
// We want to pull in nvrtc in this case
def libnvrtc = ~/libnvrtc\\.so.*\\s+=>\\s+(.*)libnvrtc.*\\.so.*\\s+.*/
def rtcm = libnvrtc.matcher(sout)
if (rtcm.find()) {
pom.properties['cuda.path'] = rtcm.group(1)
} else {
fail('could not find libnvrtc which we need to package when statically linking')
}
}
</source>
</configuration>
</execution>
Expand All @@ -426,22 +476,6 @@
<classifier>${cuda.classifier}</classifier>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>native-maven-plugin</artifactId>
<version>1.0-alpha-8</version>
<extensions>true</extensions>

<configuration>
<javahClassNames>
<javahClassName>ai.rapids.cudf.Cuda</javahClassName>
<javahClassName>ai.rapids.cudf.ColumnVector</javahClassName>
<javahClassName>ai.rapids.cudf.Table</javahClassName>
<javahClassName>ai.rapids.cudf.Rmm</javahClassName>
</javahClassNames>
<javahOutputDirectory>${basedir}/src/main/native/include/</javahOutputDirectory>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
Expand Down Expand Up @@ -479,8 +513,6 @@
<directory>${native.cudf.path}</directory>
<includes>
<include>libcudf.so</include>
<include>libNVCategory.so</include>
<include>libNVStrings.so</include>
</includes>
</resource>
<resource>
Expand Down
57 changes: 46 additions & 11 deletions java/src/main/java/ai/rapids/cudf/NativeDepsLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,54 @@
*/
public class NativeDepsLoader {
private static final Logger log = LoggerFactory.getLogger(NativeDepsLoader.class);
private static final String[] loadOrder = new String[] {
"boost_filesystem",
"rmm",
"NVStrings",
"NVCategory",
"cudf",
"cudfjni"
private static final Loader[] loadOrder = new Loader[] {
new Optional("nvrtc"),
new Required("boost_filesystem"),
new Required("rmm"),
new Required("cudf"),
new Required("cudfjni")
};
private static ClassLoader loader = NativeDepsLoader.class.getClassLoader();
private static boolean loaded = false;

private static abstract class Loader {
protected final String baseName;
public Loader(String baseName) {
this.baseName = baseName;
}

public abstract void load(String os, String arch) throws IOException;
}

private static class Required extends Loader {
public Required(String baseName) {
super(baseName);
}

@Override
public void load(String os, String arch) throws IOException {
loadDep(os, arch, this.baseName, true);
}
}

private static class Optional extends Loader {
public Optional(String baseName) {
super(baseName);
}

@Override
public void load(String os, String arch) throws IOException {
loadDep(os, arch, this.baseName, false);
}
}

static synchronized void loadNativeDeps() {
if (!loaded) {
String os = System.getProperty("os.name");
String arch = System.getProperty("os.arch");
try {
for (String toLoad : loadOrder) {
loadDep(os, arch, toLoad);
for (Loader toLoad : loadOrder) {
toLoad.load(os, arch);
}
loaded = true;
} catch (Throwable t) {
Expand All @@ -57,15 +87,20 @@ static synchronized void loadNativeDeps() {
}
}

private static void loadDep(String os, String arch, String baseName) throws IOException {
private static void loadDep(String os, String arch, String baseName, boolean required)
throws IOException {
String path = arch + "/" + os + "/" + System.mapLibraryName(baseName);
File loc;
URL resource = loader.getResource(path);
if (resource == null) {
// It looks like we are not running from the jar, or there are issues with the jar
File f = new File("./target/native-deps/" + path);
if (!f.exists()) {
throw new FileNotFoundException("Could not locate native dependency " + path);
if (required) {
throw new FileNotFoundException("Could not locate native dependency " + path);
}
// Not required so we will skip it
return;
}
resource = f.toURL();
}
Expand Down