Skip to content

Commit

Permalink
Add Windows Support
Browse files Browse the repository at this point in the history
Signed-off-by: Naveen <[email protected]>
  • Loading branch information
naveentatikonda committed Oct 24, 2022
1 parent 9d2bc9d commit ccb8149
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 3 deletions.
38 changes: 37 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
matrix:
java: [11, 17]

name: Build and Test k-NN Plugin
name: Build and Test k-NN Plugin on Ubuntu
runs-on: ubuntu-latest

steps:
Expand All @@ -42,6 +42,42 @@ jobs:
with:
token: ${{ secrets.CODECOV_TOKEN }}

Build-k-NN-Windows:
strategy:
matrix:
java: [ 11, 17 ]

name: Build and Test k-NN Plugin on Windows
runs-on: windows-latest

steps:
- name: Checkout k-NN
uses: actions/checkout@v1

- name: Setup Java ${{ matrix.java }}
uses: actions/setup-java@v1
with:
java-version: ${{ matrix.java }}

- name: Install MinGW Using Scoop
run: |
iex "& {$(irm get.scoop.sh)} -RunAsAdmin"
scoop bucket add main
scoop install mingw
- name: Add MinGW to PATH
run: |
echo "C:/Users/runneradmin/scoop/apps/mingw/current/bin" >> $env:GITHUB_PATH
refreshenv
- name: Run build
run: |
./gradlew.bat build
- name: Upload Coverage Report
uses: codecov/codecov-action@v1
with:
token: ${{ secrets.CODECOV_TOKEN }}

# - name: Pull and Run Docker for security tests
# run: |
# plugin=`ls build/distributions/*.zip`
Expand Down
11 changes: 10 additions & 1 deletion build-tools/knnplugin-coverage.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* cluster is stopped and dump it to a file. Luckily our current security policy seems to allow this. This will also probably
* break if there are multiple nodes in the integTestCluster. But for now... it sorta works.
*/

import org.apache.tools.ant.taskdefs.condition.Os
apply plugin: 'jacoco'

// Get gradle to generate the required jvm agent arg for us using a dummy tasks of type Test. Unfortunately Elastic's
Expand Down Expand Up @@ -63,7 +65,14 @@ afterEvaluate {
jacocoTestReport.dependsOn integTest

testClusters.integTest {
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/')
if (Os.isFamily(Os.FAMILY_WINDOWS)) {

// Replacing build with absolute path to fix the error "error opening zip file or JAR manifest missing : /build/tmp/expandedArchives/..../jacocoagent.jar"
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('build',"${buildDir}")
} else {
jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/')
}

systemProperty 'com.sun.management.jmxremote', "true"
systemProperty 'com.sun.management.jmxremote.authenticate', "false"
systemProperty 'com.sun.management.jmxremote.port', "7777"
Expand Down
24 changes: 23 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import org.opensearch.gradle.test.RestIntegTestTask
import org.apache.tools.ant.taskdefs.condition.Os

buildscript {
ext {
Expand Down Expand Up @@ -170,9 +171,19 @@ dependencies {
def opensearch_tmp_dir = rootProject.file('build/private/opensearch_tmp').absoluteFile
opensearch_tmp_dir.mkdirs()

task windowsPatches(type:Exec) {
commandLine 'cmd', '/c', "Powershell -File $rootDir\\scripts\\windowsScript.ps1"
}

task cmakeJniLib(type:Exec) {
workingDir 'jni'
commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}"
if (Os.isFamily(Os.FAMILY_WINDOWS)) {
dependsOn windowsPatches
commandLine 'cmake', '.', "-G", "Unix Makefiles", "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DBLAS_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DLAPACK_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll"
}
else {
commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}"
}
}

task buildJniLib(type:Exec) {
Expand All @@ -185,6 +196,11 @@ test {
dependsOn buildJniLib
systemProperty 'tests.security.manager', 'false'
systemProperty "java.library.path", "$rootDir/jni/release"
if (Os.isFamily(Os.FAMILY_WINDOWS)) {

// Add the paths of built JNI libraries and its dependent libraries to PATH variable in System variables
environment('PATH', System.getenv('PATH') + ";$rootDir/jni/release" + ";$rootDir/src/main/resources/windowsDependencies")
}
}

def _numNodes = findProperty('numNodes') as Integer ?: 1
Expand Down Expand Up @@ -225,6 +241,12 @@ integTest {
testClusters.integTest {
testDistribution = "ARCHIVE"
plugin(project.tasks.bundlePlugin.archiveFile)
if (Os.isFamily(Os.FAMILY_WINDOWS)) {

// Add the paths of built JNI libraries and its dependent libraries to PATH variable in System variables
environment('PATH', System.getenv('PATH') + ";$rootDir/jni/release" + ";$rootDir/src/main/resources/windowsDependencies")
}

// Cluster shrink exception thrown if we try to set numberOfNodes to 1, so only apply if > 1
if (_numNodes > 1) numberOfNodes = _numNodes
// When running integration tests it doesn't forward the --debug-jvm to the cluster anymore
Expand Down
119 changes: 119 additions & 0 deletions patches/windows/CMakeLists.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
Index: jni/CMakeLists.txt
<+>UTF-8
===================================================================
Description:
We are making the following changes to jni/CMakeLists.txt file to build it on Windows OS
1. Set CXX_COMPILER_VERSION and CMAKE_CXX_FLAGS.
2. Add a new if else case when the CMAKE_SYSTEM_NAME matches Windows and set flags JVM_OS_TYPE, prefix and extension for the target libraries that are built.
3. Replace LIBRARY_OUTPUT_DIRECTORY with RUNTIME_OUTPUT_DIRECTORY, to build the target libraries(opensearchknn_common, opensearchknn_nmslib and opensearchknn_faiss) in the
specified directory at runtime.
4. Comment the TESTS for now because the tests are failing(failing to build jni_tests.exe) if we are building our target libraries as SHARED libraries.

TODO: Fix the failing JNI TESTS

===================================================================
diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt
--- a/jni/CMakeLists.txt (revision 78a2b5b2a83db52aded56ab74940a62c94a88b50)
+++ b/jni/CMakeLists.txt (date 1666128659873)
@@ -14,8 +14,10 @@
set(TARGET_LIB_FAISS opensearchknn_faiss) # faiss JNI
set(TARGET_LIBS "") # Libs to be installed

+set(CXX_COMPILER_VERSION ${CMAKE_CXX_COMPILER_VERSION})
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD_REQUIRED True)
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive")

option(CONFIG_FAISS "Configure faiss library build when this is on")
option(CONFIG_NMSLIB "Configure nmslib library build when this is on")
@@ -35,6 +37,11 @@
elseif(${CMAKE_SYSTEM_NAME} STREQUAL Linux)
set(JVM_OS_TYPE linux)
set(LIB_EXT .so)
+elseif(${CMAKE_SYSTEM_NAME} STREQUAL Windows)
+ set(JVM_OS_TYPE win32)
+ set(LIB_EXT .dll)
+ set(CMAKE_SHARED_LIBRARY_PREFIX "")
+ set(CMAKE_STATIC_LIBRARY_PREFIX "")
else()
message(FATAL_ERROR "Unable to run on system: ${CMAKE_SYSTEM_NAME}")
endif()
@@ -57,7 +64,7 @@
target_include_directories(${TARGET_LIB_COMMON} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE})
set_target_properties(${TARGET_LIB_COMMON} PROPERTIES SUFFIX ${LIB_EXT})
set_target_properties(${TARGET_LIB_COMMON} PROPERTIES POSITION_INDEPENDENT_CODE ON)
-set_target_properties(${TARGET_LIB_COMMON} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)
+set_target_properties(${TARGET_LIB_COMMON} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)
list(APPEND TARGET_LIBS ${TARGET_LIB_COMMON})
# ----------------------------------------------------------------------------

@@ -79,7 +86,7 @@
target_include_directories(${TARGET_LIB_NMSLIB} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE} ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib/similarity_search/include)
set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES SUFFIX ${LIB_EXT})
set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES POSITION_INDEPENDENT_CODE ON)
- set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)
+ set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)

list(APPEND TARGET_LIBS ${TARGET_LIB_NMSLIB})
endif ()
@@ -130,7 +137,7 @@
target_include_directories(${TARGET_LIB_FAISS} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE} ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss)
set_target_properties(${TARGET_LIB_FAISS} PROPERTIES SUFFIX ${LIB_EXT})
set_target_properties(${TARGET_LIB_FAISS} PROPERTIES POSITION_INDEPENDENT_CODE ON)
- set_target_properties(${TARGET_LIB_FAISS} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)
+ set_target_properties(${TARGET_LIB_FAISS} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release)

list(APPEND TARGET_LIBS ${TARGET_LIB_FAISS})
endif ()
@@ -138,51 +145,7 @@
# ---------------------------------------------------------------------------

# --------------------------------- TESTS -----------------------------------
-if (${CONFIG_ALL} STREQUAL ON OR ${CONFIG_TEST} STREQUAL ON)
- # Reference - https://crascit.com/2015/07/25/cmake-gtest/
- configure_file(CMakeLists.txt.in googletest-download/CMakeLists.txt)
- execute_process(COMMAND "${CMAKE_COMMAND}" -G "${CMAKE_GENERATOR}" .
- WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/googletest-download"
- )
- execute_process(COMMAND "${CMAKE_COMMAND}" --build .
- WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/googletest-download"
- )
- set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
-
- add_subdirectory("${CMAKE_BINARY_DIR}/googletest-src"
- "${CMAKE_BINARY_DIR}/googletest-build" EXCLUDE_FROM_ALL
- )
- add_executable(
- jni_test
- tests/faiss_wrapper_test.cpp
- tests/nmslib_wrapper_test.cpp
- tests/test_util.cpp)
-
- target_link_libraries(
- jni_test
- gtest_main
- gmock_main
- faiss
- NonMetricSpaceLib
- OpenMP::OpenMP_CXX
- ${TARGET_LIB_FAISS}
- ${TARGET_LIB_NMSLIB}
- ${TARGET_LIB_COMMON}
- )
-
- target_include_directories(jni_test PRIVATE
- ${CMAKE_CURRENT_SOURCE_DIR}/tests
- ${CMAKE_CURRENT_SOURCE_DIR}/include
- $ENV{JAVA_HOME}/include
- $ENV{JAVA_HOME}/include/${JVM_OS_TYPE}
- ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss
- ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib/similarity_search/include
- ${gtest_SOURCE_DIR}/include
- ${gmock_SOURCE_DIR}/include)

-
- set_target_properties(jni_test PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/bin)
-endif ()
# ---------------------------------------------------------------------------

# -------------------------------- INSTALL ----------------------------------
23 changes: 23 additions & 0 deletions scripts/windowsScript.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#
# Copyright OpenSearch Contributors
# SPDX-License-Identifier: Apache-2.0
#

git submodule update --init -- jni/external/nmslib
git submodule update --init -- jni/external/faiss

git apply patches/windows/CMakeLists.patch --verbose

# Validating if the CMakeLists patch is applied
echo "Validating if the CMakeLists patch is applied"
type jni/CMakeLists.txt | Select-String "Windows"

# Replace '_MSC_VER' with '__MINGW32__'
(Get-Content jni/external/faiss/faiss/impl/index_read.cpp).replace('_MSC_VER', '__MINGW32__') | Set-Content jni/external/faiss/faiss/impl/index_read.cpp
(Get-Content jni/external/faiss/faiss/impl/index_write.cpp).replace('_MSC_VER', '__MINGW32__') | Set-Content jni/external/faiss/faiss/impl/index_write.cpp

# Replace '#include <sys/mman.h>' with
# #ifndef __MINGW32__
# #include <sys/mman.h>
# #endif
(Get-Content jni/external/faiss/faiss/OnDiskInvertedLists.cpp).replace('#include <sys/mman.h>', "#ifndef __MINGW32__`n#include <sys/mman.h>`n#endif") | Set-Content jni/external/faiss/faiss/OnDiskInvertedLists.cpp
Binary file not shown.

0 comments on commit ccb8149

Please sign in to comment.