Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

[MXNET-57] Added support android64 #11055

Merged
marcoabreu merged 1 commit into from
Jun 15, 2018
Merged

[MXNET-57] Added support android64 #11055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mxnet_option(USE_NCCL "Use NVidia NCCL with CUDA" OFF)
mxnet_option(USE_OPENCV "Build with OpenCV support" ON)
mxnet_option(USE_OPENMP "Build with Openmp support" ON)
mxnet_option(USE_CUDNN "Build with cudnn support" ON) # one could set CUDNN_ROOT for search path
mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON)
mxnet_option(USE_SSE "Build with x86 SSE instruction support" ON IF NOT ARM)
mxnet_option(USE_F16C "Build with x86 F16C instruction support" ON) # autodetects support if ON
Copy link
Copy Markdown
Member

@szha szha Jun 7, 2018

Choose a reason for hiding this comment

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

does ARM/android support F16C instruction?

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 7, 2018

Choose a reason for hiding this comment

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

AFAIK F16C is x86, so maybe only in android x86 but most android is ARM, and this is an ARM build ATM. We should add x86 android builds in the future, good point.

Copy link
Copy Markdown
Contributor

@lebeg lebeg Jun 7, 2018

Choose a reason for hiding this comment

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

We should do an automatic scan in the future if the feature is present on the system and the compiler like for F16 in this PR.

Copy link
Copy Markdown
Contributor

@lebeg lebeg Jun 11, 2018

Choose a reason for hiding this comment

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

Does this work also for cross compilation?

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 12, 2018

Choose a reason for hiding this comment

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

Can you give thumbs up or down to the review? Any more comments?

Copy link
Copy Markdown
Contributor

@lebeg lebeg Jun 12, 2018

Choose a reason for hiding this comment

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

Sure, here you go)

mxnet_option(USE_LAPACK "Build with lapack support" ON)
mxnet_option(USE_MKL_IF_AVAILABLE "Use MKL if found" ON)
Expand Down Expand Up @@ -360,7 +360,7 @@ if(USE_OPENMP)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
endif()
endif()
elseif(UNIX)
elseif(UNIX AND NOT ANDROID)
list(APPEND mxnet_LINKER_LIBS pthread)
endif()

Expand Down
10 changes: 10 additions & 0 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,16 @@ try {
}
}
}
},
'Android / ARM64':{
node('mxnetlinux-cpu') {
ws('workspace/android64') {
timeout(time: max_time, unit: 'MINUTES') {
init_git()
docker_run('android_arm64', 'build_android_arm64', false)
}
}
}
}
} // End of stage('Build')

Expand Down
4 changes: 1 addition & 3 deletions ci/docker/Dockerfile.build.android_arm64
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ MAINTAINER Pedro Larroy "pllarroy@amazon.com"
# extract ccache binary into latest context
COPY --from=ccachebuilder /usr/local/bin/ccache /usr/local/bin/ccache

# The cross-compiling emulator
RUN apt-get update && apt-get install -y \
qemu-user \
qemu-user-static \
unzip

ENV CROSS_TRIPLE=aarch64-linux-android
Expand All @@ -61,6 +58,7 @@ LABEL org.label-schema.build-date=$BUILD_DATE \
org.label-schema.schema-version="1.0"

ENV ARCH aarch64
ENV ANDROID_NDK_REVISION 15c

ENV CC=${CROSS_ROOT}/bin/${CROSS_TRIPLE}-clang
ENV CXX=${CROSS_ROOT}/bin/${CROSS_TRIPLE}-clang++
Expand Down
5 changes: 3 additions & 2 deletions ci/docker/install/android_arm64_ndk.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

set -ex
pushd .
export ANDROID_NDK_REVISION=15c
# This environment variable comes from the docker file
echo "Downloading android SDK rev ${ANDROID_NDK_REVISION}"
curl -O https://dl.google.com/android/repository/android-ndk-r${ANDROID_NDK_REVISION}-linux-x86_64.zip && \
unzip ./android-ndk-r${ANDROID_NDK_REVISION}-linux-x86_64.zip && \
cd android-ndk-r${ANDROID_NDK_REVISION} && \
Expand All @@ -32,4 +33,4 @@ cd android-ndk-r${ANDROID_NDK_REVISION} && \
--api 21 \
--install-dir=${CROSS_ROOT} && \

popd
popd
32 changes: 29 additions & 3 deletions ci/docker/runtime_functions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,41 +173,67 @@ build_arm64() {
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DUSE_CUDA=OFF\
-DSUPPORT_F16C=OFF\
-DUSE_OPENCV=OFF\
-DUSE_OPENMP=OFF\
-DUSE_SIGNAL_HANDLER=ON\
-DCMAKE_BUILD_TYPE=RelWithDebInfo\
-DUSE_MKL_IF_AVAILABLE=OFF\
-G Ninja /work/mxnet
ninja
ninja -v
export MXNET_LIBRARY_PATH=`pwd`/libmxnet.so
cd /work/mxnet/python
python setup.py bdist_wheel --universal
cp dist/*.whl /work/build
}

build_android_arm64() {
build_android_armv7() {
set -ex
cd /work/build
cmake \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DUSE_CUDA=OFF\
-DUSE_SSE=OFF\
-DSUPPORT_F16C=OFF\
-DUSE_LAPACK=OFF\
-DUSE_OPENCV=OFF\
-DUSE_OPENMP=OFF\
-DUSE_SIGNAL_HANDLER=ON\
-DCMAKE_BUILD_TYPE=RelWithDebInfo\
-DUSE_MKL_IF_AVAILABLE=OFF\
-G Ninja /work/mxnet
ninja
ninja -v
export MXNET_LIBRARY_PATH=`pwd`/libmxnet.so
cd /work/mxnet/python
python setup.py bdist_wheel --universal
cp dist/*.whl /work/build
}

build_android_arm64() {
set -ex
cd /work/build
# There are other ways for CMake to cross compile android, like setting the following variables
# below. But right not it doesn't work as expected, we need to find what's the best strategy to
# build with CMake in Android.
# -DCMAKE_ANDROID_NDK=${CROSS_ROOT} \
# -DCMAKE_SYSTEM_VERSION=${ANDROID_NDK_REVISION} \
# -DCMAKE_SYSTEM_NAME=Android \
#
cmake\
-DANDROID=ON \
-DUSE_CUDA=OFF\
-DUSE_SSE=OFF\
-DUSE_LAPACK=OFF\
-DUSE_OPENCV=OFF\
-DUSE_OPENMP=OFF\
Copy link
Copy Markdown
Contributor

@marcoabreu marcoabreu Jun 6, 2018

Choose a reason for hiding this comment

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

No open mp? Android phones usually have around 4 to 8 cores, so we should have it enabled

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 7, 2018

Choose a reason for hiding this comment

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

first we make a baseline version run, don't propose to overcomplicate things. Feel free to work on making openmp work in android to use all your cores if you have time.

Copy link
Copy Markdown
Contributor

@marcoabreu marcoabreu Jun 7, 2018

Choose a reason for hiding this comment

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

Well we want to make a good first impression if people use it. If everything is slow the first time they test it, they most likely will not come back. OpenMP is very important, especially since we don't have any other hardware acceleration. Until that works, we can't really tell any customer to use it or we will lose creditability.

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 11, 2018

Choose a reason for hiding this comment

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

This is an initial build, what you are saying doesn't apply. Nobody should test this.

-DUSE_SIGNAL_HANDLER=ON\
-DCMAKE_BUILD_TYPE=RelWithDebInfo\
Copy link
Copy Markdown
Contributor

@marcoabreu marcoabreu Jun 6, 2018

Choose a reason for hiding this comment

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

To reduce the size, I'd propose we build without debug info for now

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 7, 2018

Choose a reason for hiding this comment

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

I want symbols to diagnose initial runs and crashes, there's a reason for this.

Copy link
Copy Markdown
Contributor

@marcoabreu marcoabreu Jun 7, 2018

Choose a reason for hiding this comment

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

I'd propose that you set that flag locally then. We're referencing these instructions in our guides and to our customers, means we have to provide a ready-made solution. We had the same case previously when a customer complained about the size and in fact it was only the RelWithDebInfo.

Copy link
Copy Markdown
Contributor Author

@larroy larroy Jun 13, 2018

Choose a reason for hiding this comment

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

I don't see how the points you are making about an experimental build which is not official nor tested are of any relevance at this stage of Android support. This is a development build and I want debug symbols to diagnose crashes, this should be reason enough to have it as it is, because size of build doesn't really matter right now, and debugability is more important.

Copy link
Copy Markdown
Contributor

@marcoabreu marcoabreu Jun 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We already spoke about this offline and I said that I'm fine with moving forward. Before you explaining it to me, I was under the impression that this would be the final stage.

-DUSE_MKL_IF_AVAILABLE=OFF\
-G Ninja /work/mxnet
ninja -v
}

build_centos7_cpu() {
set -ex
cd /work/mxnet
Expand Down
5 changes: 3 additions & 2 deletions src/operator/random/shuffle_op.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
* \file shuffle_op.cc
* \brief Operator to shuffle elements of an NDArray
*/
#if (__GNUC__ > 4 && !defined(__clang__major__)) || (__clang_major__ > 4 && __linux__)
#define USE_GNU_PARALLEL_SHUFFLE
#if !defined (__ANDROID__) && ((__GNUC__ > 4 &&\
!defined(__clang__major__)) || (__clang_major__ > 4 && __linux__))
#define USE_GNU_PARALLEL_SHUFFLE
#endif

#include <mxnet/operator_util.h>
Expand Down