From 445029bb82ff5c52383b741a161a0c03b745529e Mon Sep 17 00:00:00 2001 From: Adam Pocock Date: Tue, 18 Oct 2016 08:35:25 -0400 Subject: [PATCH] [jvm-packages] XGBoost4j Windows fixes (#1639) * 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 --- CONTRIBUTORS.md | 1 + Makefile | 1 + include/xgboost/base.h | 2 +- include/xgboost/c_api.h | 10 +- jvm-packages/create_jni.bat | 11 +- jvm-packages/xgboost4j/pom.xml | 122 +++++++++++++----- .../xgboost4j/src/native/xgboost4j.cpp | 26 ++-- 7 files changed, 120 insertions(+), 53 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index e03a15b53efa..2c3039494bf2 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -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) diff --git a/Makefile b/Makefile index 27c45b6aca81..82f425a88426 100644 --- a/Makefile +++ b/Makefile @@ -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) diff --git a/include/xgboost/base.h b/include/xgboost/base.h index a3b8c31f53e6..ac745cbaf4c7 100644 --- a/include/xgboost/base.h +++ b/include/xgboost/base.h @@ -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; diff --git a/include/xgboost/c_api.h b/include/xgboost/c_api.h index 464fa280c3cd..36c64c5d0751 100644 --- a/include/xgboost/c_api.h +++ b/include/xgboost/c_api.h @@ -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; @@ -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 */ diff --git a/jvm-packages/create_jni.bat b/jvm-packages/create_jni.bat index cbc0681c18db..596374e30164 100644 --- a/jvm-packages/create_jni.bat +++ b/jvm-packages/create_jni.bat @@ -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 diff --git a/jvm-packages/xgboost4j/pom.xml b/jvm-packages/xgboost4j/pom.xml index 1791d7a2724e..0efddd5dd96f 100644 --- a/jvm-packages/xgboost4j/pom.xml +++ b/jvm-packages/xgboost4j/pom.xml @@ -11,42 +11,96 @@ xgboost4j 0.7 jar - - - - org.apache.maven.plugins - maven-javadoc-plugin - 2.10.3 - - protected - true - - - - org.apache.maven.plugins - maven-assembly-plugin - - false - - - - exec-maven-plugin - org.codehaus.mojo - - - native - generate-sources - - exec - + + + NotWindows + + + !windows + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + 2.10.3 - create_jni.sh + protected + true - - - - - + + + org.apache.maven.plugins + maven-assembly-plugin + + false + + + + exec-maven-plugin + org.codehaus.mojo + + + native + generate-sources + + exec + + + create_jni.sh + + + + + + + + + Windows + + + windows + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + 2.10.3 + + protected + true + + + + org.apache.maven.plugins + maven-assembly-plugin + + false + + + + exec-maven-plugin + org.codehaus.mojo + + + native + generate-sources + + exec + + + create_jni.bat + + + + + + + + junit diff --git a/jvm-packages/xgboost4j/src/native/xgboost4j.cpp b/jvm-packages/xgboost4j/src/native/xgboost4j.cpp index 9df04e54e781..db4f93b440f6 100644 --- a/jvm-packages/xgboost4j/src/native/xgboost4j.cpp +++ b/jvm-packages/xgboost4j/src/native/xgboost4j.cpp @@ -12,6 +12,7 @@ limitations under the License. */ +#include #include #include #include @@ -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); } @@ -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()"; @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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; } @@ -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; } @@ -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; } @@ -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; }