Skip to content

Commit

Permalink
[jvm-packages] XGBoost4j Windows fixes (#1639)
Browse files Browse the repository at this point in the history 
* Changes for Mingw64 compilation to ensure long is a consistent size.

Mainly impacts the Java API which would not compile, but there may be
silent errors on Windows with large datasets before this patch (as long
is 32-bits when compiled with mingw64 even in 64-bit mode).

* Adding ifdefs to ensure it still compiles on MacOS

* Makefile and create_jni.bat changes for Windows.

* Switching XGDMatrixCreateFromCSREx JNI call to use size_t cast

* Fixing lint error, adding profile switching to jvm-packages build to make create-jni.bat get called, adding myself to Contributors.Md
  • Loading branch information
@Craigacp @CodingCat
Craigacp authored and CodingCat committed Oct 18, 2016
1 parent be90deb commit 445029b
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 53 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@ List of Contributors
* [Damien Carol](https://github.com/damiencarol)
* [Alex Bain](https://github.com/convexquad)
* [Baltazar Bieniek](https://github.com/bbieniek)
* [Adam Pocock](https://github.com/Craigacp)
1 change: 1 addition & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ ifneq ($(UNAME), Windows)
XGBOOST_DYLIB = lib/libxgboost.so
else
XGBOOST_DYLIB = lib/libxgboost.dll
JAVAINCFLAGS += -I${JAVA_HOME}/include/win32
endif

ifeq ($(UNAME), Linux)
Expand Down
2 changes: 1 addition & 1 deletion include/xgboost/base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace xgboost {
*/
typedef uint32_t bst_uint;
/*! \brief long integers */
typedef unsigned long bst_ulong; // NOLINT(*)
typedef uint64_t bst_ulong; // NOLINT(*)
/*! \brief float type, used for storing statistics */
typedef float bst_float;

Expand Down
10 changes: 8 additions & 2 deletions include/xgboost/c_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ XGB_EXTERN_C {
#endif

// manually define unsign long
typedef unsigned long bst_ulong; // NOLINT(*)
typedef uint64_t bst_ulong; // NOLINT(*)

/*! \brief handle to DMatrix */
typedef void *DMatrixHandle;
Expand All @@ -40,7 +40,13 @@ typedef struct {
/*! \brief number of rows in the minibatch */
size_t size;
/*! \brief row pointer to the rows in the data */
long* offset; // NOLINT(*)
#ifdef __APPLE__
/* Necessary as Java on MacOS defines jlong as long int
* and gcc defines int64_t as long long int. */
long* offset; // NOLINT(*)
#else
int64_t* offset; // NOLINT(*)
#endif
/*! \brief labels of each instance */
float* label;
/*! \brief weight of each instance, can be NULL */
Expand Down
11 changes: 5 additions & 6 deletions jvm-packages/create_jni.bat
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
echo "move native library"
set libsource=..\windows\x64\Release\xgboost4j.dll
echo "copy native library"
set libsource=..\lib\libxgboost4j.so

if not exist %libsource% (
goto end
)

set libfolder=xgboost4j\src\main\resources\lib
set libfolder=src\main\resources\lib
set libpath=%libfolder%\xgboost4j.dll
if not exist %libfolder% (mkdir %libfolder%)
if exist %libpath% (del %libpath%)
move %libsource% %libfolder%
copy %libsource% %libpath%
echo complete
pause
exit

:end
echo "source library not found, please build it first from ..\windows\xgboost.sln"
echo "source library not found, please build it first by runing mingw32-make jvm"
pause
exit
122 changes: 88 additions & 34 deletions jvm-packages/xgboost4j/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,42 +11,96 @@
<artifactId>xgboost4j</artifactId>
<version>0.7</version>
<packaging>jar</packaging>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>2.10.3</version>
<configuration>
<show>protected</show>
<nohelp>true</nohelp>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
<configuration>
<skipAssembly>false</skipAssembly>
</configuration>
</plugin>
<plugin>
<artifactId>exec-maven-plugin</artifactId>
<groupId>org.codehaus.mojo</groupId>
<executions>
<execution><!-- Run our version calculation script -->
<id>native</id>
<phase>generate-sources</phase>
<goals>
<goal>exec</goal>
</goals>
<profiles>
<profile>
<id>NotWindows</id>
<activation>
<os>
<family>!windows</family>
</os>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>2.10.3</version>
<configuration>
<executable>create_jni.sh</executable>
<show>protected</show>
<nohelp>true</nohelp>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
<configuration>
<skipAssembly>false</skipAssembly>
</configuration>
</plugin>
<plugin>
<artifactId>exec-maven-plugin</artifactId>
<groupId>org.codehaus.mojo</groupId>
<executions>
<execution><!-- Run our version calculation script -->
<id>native</id>
<phase>generate-sources</phase>
<goals>
<goal>exec</goal>
</goals>
<configuration>
<executable>create_jni.sh</executable>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>Windows</id>
<activation>
<os>
<family>windows</family>
</os>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>2.10.3</version>
<configuration>
<show>protected</show>
<nohelp>true</nohelp>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
<configuration>
<skipAssembly>false</skipAssembly>
</configuration>
</plugin>
<plugin>
<artifactId>exec-maven-plugin</artifactId>
<groupId>org.codehaus.mojo</groupId>
<executions>
<execution><!-- Run our version calculation script -->
<id>native</id>
<phase>generate-sources</phase>
<goals>
<goal>exec</goal>
</goals>
<configuration>
<executable>create_jni.bat</executable>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
<dependencies>
<dependency>
<groupId>junit</groupId>
Expand Down
26 changes: 16 additions & 10 deletions jvm-packages/xgboost4j/src/native/xgboost4j.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
limitations under the License.
*/

#include <cstdint>
#include <xgboost/c_api.h>
#include <xgboost/base.h>
#include <xgboost/logging.h>
Expand All @@ -23,7 +24,11 @@
// helper functions
// set handle
void setHandle(JNIEnv *jenv, jlongArray jhandle, void* handle) {
#ifdef __APPLE__
long out = (long) handle;
#else
int64_t out = (int64_t) handle;
#endif
jenv->SetLongArrayRegion(jhandle, 0, 1, &out);
}

Expand Down Expand Up @@ -87,7 +92,7 @@ XGB_EXTERN_C int XGBoost4jCallbackDataIterNext(
cbatch.weight = nullptr;
}
long max_elem = cbatch.offset[cbatch.size];
cbatch.index = jenv->GetIntArrayElements(jindex, 0);
cbatch.index = (int*) jenv->GetIntArrayElements(jindex, 0);
cbatch.value = jenv->GetFloatArrayElements(jvalue, 0);
CHECK_EQ(jenv->GetArrayLength(jindex), max_elem)
<< "batch.index.length must equal batch.offset.back()";
Expand All @@ -107,7 +112,7 @@ XGB_EXTERN_C int XGBoost4jCallbackDataIterNext(
jenv->ReleaseFloatArrayElements(jweight, cbatch.weight, 0);
jenv->DeleteLocalRef(jweight);
}
jenv->ReleaseIntArrayElements(jindex, cbatch.index, 0);
jenv->ReleaseIntArrayElements(jindex, (jint*) cbatch.index, 0);
jenv->DeleteLocalRef(jindex);
jenv->ReleaseFloatArrayElements(jvalue, cbatch.value, 0);
jenv->DeleteLocalRef(jvalue);
Expand Down Expand Up @@ -199,7 +204,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_XGDMatrixCreateFro
jfloat* data = jenv->GetFloatArrayElements(jdata, 0);
bst_ulong nindptr = (bst_ulong)jenv->GetArrayLength(jindptr);
bst_ulong nelem = (bst_ulong)jenv->GetArrayLength(jdata);
int ret = (jint) XGDMatrixCreateFromCSREx((unsigned long const *)indptr, (unsigned int const *)indices, (float const *)data, nindptr, nelem, jcol, &result);
jint ret = (jint) XGDMatrixCreateFromCSREx((size_t const *)indptr, (unsigned int const *)indices, (float const *)data, nindptr, nelem, jcol, &result);
setHandle(jenv, jout, result);
//Release
jenv->ReleaseLongArrayElements(jindptr, indptr, 0);
Expand All @@ -222,7 +227,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_XGDMatrixCreateFro
bst_ulong nindptr = (bst_ulong)jenv->GetArrayLength(jindptr);
bst_ulong nelem = (bst_ulong)jenv->GetArrayLength(jdata);

int ret = (jint) XGDMatrixCreateFromCSCEx((unsigned long const *)indptr, (unsigned int const *)indices, (float const *)data, nindptr, nelem, jrow, &result);
jint ret = (jint) XGDMatrixCreateFromCSCEx((size_t const *)indptr, (unsigned int const *)indices, (float const *)data, nindptr, nelem, jrow, &result);
setHandle(jenv, jout, result);
//release
jenv->ReleaseLongArrayElements(jindptr, indptr, 0);
Expand All @@ -244,7 +249,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_XGDMatrixCreateFro
jfloat* data = jenv->GetFloatArrayElements(jdata, 0);
bst_ulong nrow = (bst_ulong)jnrow;
bst_ulong ncol = (bst_ulong)jncol;
int ret = (jint) XGDMatrixCreateFromMat((float const *)data, nrow, ncol, jmiss, &result);
jint ret = (jint) XGDMatrixCreateFromMat((float const *)data, nrow, ncol, jmiss, &result);
setHandle(jenv, jout, result);
//release
jenv->ReleaseFloatArrayElements(jdata, data, 0);
Expand All @@ -264,7 +269,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_XGDMatrixSliceDMat
jint* indexset = jenv->GetIntArrayElements(jindexset, 0);
bst_ulong len = (bst_ulong)jenv->GetArrayLength(jindexset);

int ret = XGDMatrixSliceDMatrix(handle, (int const *)indexset, len, &result);
jint ret = (jint) XGDMatrixSliceDMatrix(handle, (int const *)indexset, len, &result);
setHandle(jenv, jout, result);
//release
jenv->ReleaseIntArrayElements(jindexset, indexset, 0);
Expand Down Expand Up @@ -650,7 +655,8 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_XGBoosterLoadRabit
BoosterHandle handle = (BoosterHandle) jhandle;
int version;
int ret = XGBoosterLoadRabitCheckpoint(handle, &version);
jenv->SetIntArrayRegion(jout, 0, 1, &version);
jint jversion = version;
jenv->SetIntArrayRegion(jout, 0, 1, &jversion);
return ret;
}

Expand Down Expand Up @@ -722,7 +728,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitTrackerPrint
*/
JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitGetRank
(JNIEnv *jenv, jclass jcls, jintArray jout) {
int rank = RabitGetRank();
jint rank = RabitGetRank();
jenv->SetIntArrayRegion(jout, 0, 1, &rank);
return 0;
}
Expand All @@ -734,7 +740,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitGetRank
*/
JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitGetWorldSize
(JNIEnv *jenv, jclass jcls, jintArray jout) {
int out = RabitGetWorldSize();
jint out = RabitGetWorldSize();
jenv->SetIntArrayRegion(jout, 0, 1, &out);
return 0;
}
Expand All @@ -746,7 +752,7 @@ JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitGetWorldSize
*/
JNIEXPORT jint JNICALL Java_ml_dmlc_xgboost4j_java_XGBoostJNI_RabitVersionNumber
(JNIEnv *jenv, jclass jcls, jintArray jout) {
int out = RabitVersionNumber();
jint out = RabitVersionNumber();
jenv->SetIntArrayRegion(jout, 0, 1, &out);
return 0;
}

0 comments on commit 445029b

Please sign in to comment.