Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Add arm64 As Supported ABI #1956

Closed
bleege opened this issue Jul 28, 2015 · 56 comments
Closed

Add arm64 As Supported ABI #1956

bleege opened this issue Jul 28, 2015 · 56 comments
Assignees
Labels
Android Mapbox Maps SDK for Android build

Comments

@bleege
Copy link
Contributor

bleege commented Jul 28, 2015

Nexus 9 devices are arm64 based so Android GL needs to provide a arm64 based libmapboxgl.so binding. Update build process to support this ABI.

https://developer.android.com/ndk/guides/abis.html

/cc @incanus @ljbade

@bleege bleege added the Android Mapbox Maps SDK for Android label Jul 28, 2015
@bleege bleege added this to the Android b1 milestone Jul 28, 2015
@bleege
Copy link
Contributor Author

bleege commented Jul 28, 2015

Added make android-lib-arm64-v8a to ANDROID_ABIS to get the process started. When runs it produces:

Brads-MacBook-Pro:mapbox-gl-android brad$ make android-lib-arm64-v8a
git submodule update --init src/mbgl/util/geojsonvt
deps/run_gyp android/mapboxgl-app.gyp -Dhost=android -Iconfig/android-arm64-v8a.gypi -Dheadless_lib=none -Dplatform_lib=android -Dasset_lib=zip -Dhttp_lib=curl -Dcache_lib=sqlite --depth=. -Goutput_dir=. --generator-output=./build/android-arm64-v8a -f make-android
gyp: Undefined variable openssl_ldflags in android/mapboxgl-app.gyp
make: *** [Makefile/android-arm64-v8a] Error 1

@bleege
Copy link
Contributor Author

bleege commented Jul 28, 2015

Discovered that the android-arm64-v8a.gypi is missing openssl_ldflags variable compared to what it looks like in android-arm-v7.gypi

android-arm64-v8a.gypi

# Do not edit. Generated by the configure script.
{
  'target_defaults': {
    'cflags%': [],
    'default_configuration': 'Release',
    'defines': [],
    'include_dirs': [],
    'libraries': []
  },
  'variables': {
    'boost_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/headers/boost/1.57.0/include'],
    'boost_program_options_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/boost_libprogram_options/1.57.0/lib/libboost_program_options.a'],
    'curl_static_libs%': [],
    'curl_cflags%': ['-isysroot', '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk', '-arch', 'x86_64', '-mmacosx-version-min=10.8', '-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libcurl/system/include'],
    'curl_ldflags%': ['-Wl,-search_paths_first', '-isysroot', '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk', '-arch', 'x86_64', '-mmacosx-version-min=10.8', '-L/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libcurl/system/lib', '-lcurl'],
    'glfw3_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/glfw/3.1/lib/libglfw3.a'],
    'glfw3_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/glfw/3.1/include'],
    'glfw3_ldflags%': ['-framework Cocoa', '-framework OpenGL', '-framework IOKit', '-framework CoreFoundation', '-framework CoreVideo'],
    'opengl_cflags%': [],
    'opengl_ldflags%': [],
    'png_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libpng/1.6.16/lib/libpng.a'],
    'png_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libpng/1.6.16/include/libpng16'],
    'png_ldflags%': ['-lz'],
    'jpeg_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/jpeg/v9a/lib/libjpeg.a'],
    'jpeg_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/jpeg/v9a/include'],
    'jpeg_ldflags%': [],
    'sqlite3_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/sqlite/3.8.8.1/lib/libsqlite3.a'],
    'sqlite3_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/sqlite/3.8.8.1/include'],
    'sqlite3_ldflags%': [],
    'uv_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libuv/0.10.28/lib/libuv.a'],
    'uv_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libuv/0.10.28/include'],
    'uv_ldflags%': ['-lpthread', '-ldl', '-framework CoreFoundation', '-framework CoreServices'],
    'zlib_static_libs%': [],
    'zlib_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/zlib/system/include'],
    'zlib_ldflags%': ['-L/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/zlib/system/lib', '-lz'],
    'nu_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/nunicode/1.5.1/lib/libnu.a'],
    'nu_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/nunicode/1.5.1/include'],
    'nu_ldflags%': [],
    'zip_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libzip/0.11.2/lib/libzip.a'],
    'zip_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libzip/0.11.2/include', '-I/Users/brad/development/mapbox-gl-android/mason_packages/osx-10.10/libzip/0.11.2/lib/libzip/include'],
    'zip_ldflags%': ['-lz'],
  }
}

android-arm-v7.gypi

# Do not edit. Generated by the configure script.
{
  'target_defaults': {
    'cflags%': [],
    'default_configuration': 'Release',
    'defines': [],
    'include_dirs': [],
    'libraries': []
  },
  'variables': {
    'boost_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/headers/boost/1.57.0/include'],
    'openssl_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/openssl/1.0.1l/lib/libssl.a'],
    'openssl_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/openssl/1.0.1l/include'],
    'openssl_ldflags%': ['-L/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/openssl/1.0.1l/lib', '-lssl', '-lcrypto', '-Wl,--fix-cortex-a8', '-ldl'],
    'curl_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libcurl/7.40.0/lib/libcurl.a'],
    'curl_cflags%': ['-DCURL_STATICLIB', '-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libcurl/7.40.0/include'],
    'curl_ldflags%': ['-L/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libcurl/7.40.0/lib', '-lcurl', '-lssl', '-lcrypto', '-lssl', '-lcrypto', '-lz'],
    'opengl_cflags%': [],
    'opengl_ldflags%': [],
    'png_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libpng/1.6.16/lib/libpng.a'],
    'png_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libpng/1.6.16/include/libpng16'],
    'png_ldflags%': ['-lz', '-lm'],
    'jpeg_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/jpeg/v9a/lib/libjpeg.a'],
    'jpeg_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/jpeg/v9a/include'],
    'jpeg_ldflags%': [],
    'sqlite3_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/sqlite/3.8.8.1/lib/libsqlite3.a'],
    'sqlite3_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/sqlite/3.8.8.1/include'],
    'sqlite3_ldflags%': [],
    'uv_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libuv/1.4.0/lib/libuv.a'],
    'uv_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libuv/1.4.0/include'],
    'uv_ldflags%': ['-ldl'],
    'zlib_static_libs%': [],
    'zlib_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/zlib/system/include'],
    'zlib_ldflags%': ['-L/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/zlib/system/lib', '-lz'],
    'nu_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/nunicode/1.5.1/lib/libnu.a'],
    'nu_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/nunicode/1.5.1/include'],
    'nu_ldflags%': [],
    'zip_static_libs%': ['/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libzip/0.11.2/lib/libzip.a'],
    'zip_cflags%': ['-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libzip/0.11.2/include', '-I/Users/brad/development/mapbox-gl-android/mason_packages/android-arm-v7-9/libzip/0.11.2/lib/libzip/include'],
    'zip_ldflags%': ['-lz'],
  }
}

@bleege
Copy link
Contributor Author

bleege commented Jul 28, 2015

Looking at differences between two files ^^, it appears that the android option in the configure files isn't being run for android-arm64-v8a like it is for regular android-arm-v7. Instead it's falling through to the wildcard option which means that different libraries are being defined for inclusion. Solving this should get things back on track for Android.

@ljbade
Copy link
Contributor

ljbade commented Jul 28, 2015

I will take a look. I think arm64 used to build OK.

@bleege
Copy link
Contributor Author

bleege commented Jul 28, 2015

The issue is that the make android-lib-% target is not being triggered when make android-lib-arm64-v8a is called, but is being triggered when make android-lib-arm-v7 is called. MASON_PLATFORM is set by the commands inside this trigger.

make android-lib-arm-v7
20150728-make-android-working

make android-lib-arm64-v8a
20150728-make-android-not-working

@ljbade
Copy link
Contributor

ljbade commented Jul 28, 2015

@bleege Ah that triggered my memory see #1684

@bleege
Copy link
Contributor Author

bleege commented Jul 29, 2015

@ljbade Interesting. Yeah, I just swapped out the default value for ANDROID_ABI of arm-v7 to arm64-v8a and ran make android and it errored out the exact same way again. This seems like it's a Makefile pattern matching for targets issue. I asked @flippmoke about it and he couldn't see the issue either. Will defer to @kkaefer on this.

Why this issue is important:

  1. Android GL needs to run on hardware with this architecture for Mapbox customers
  2. @incanus current development device is a Nexus 9 that runs on this.

@ljbade ljbade added the build label Jul 29, 2015
@ljbade
Copy link
Contributor

ljbade commented Aug 6, 2015

@bleege has @kkaefer refactor of the build fixed this?

@ljbade
Copy link
Contributor

ljbade commented Aug 7, 2015

Just noticed on Travis that in the build matrix, irrespective of what export ANDROID_ABI= is set to it always builds ARM v7 even in the ARM v8 and x86 jobs.

@kkaefer Does the build system still listen to export ANDROID_ABI=?

@ljbade
Copy link
Contributor

ljbade commented Aug 7, 2015

@bleege Did some tests. On current master make android-all successfully builds all the JNI libs including ARM v8.

Can you try that command and then see if you can roll a ARM v8 snapshot for @incanus to test on his 64-bit device?

I am currently installing an ARM v8 virtual device to give it a quick test too.

Also working on fixing the Travis build to correctly build each ABI.

@ljbade
Copy link
Contributor

ljbade commented Aug 7, 2015

@bleege after make android-all you should see all these in your jniLibs folder:
image

@ljbade
Copy link
Contributor

ljbade commented Aug 7, 2015

The ARM v8 emulator won't launch on my computer for some reason.

@bleege @incanus Can you test it for me?

@ljbade
Copy link
Contributor

ljbade commented Aug 13, 2015

No longer a problem after build system was refactored.

@bleege
Copy link
Contributor Author

bleege commented Aug 19, 2015

Reopening as we've had reports of the app crashing immediately on devices that have 64 bit processors. Specifically:

  • Samsung S6
  • Nexus 9
  • HTC One M9

/cc @incanus @bsudekum @tcql

@tcql
Copy link

tcql commented Aug 19, 2015

be sure to let me know if there's any debug info I can capture that will help nail this down

@ljbade
Copy link
Contributor

ljbade commented Aug 20, 2015

Let me fire up the simulator

@ljbade
Copy link
Contributor

ljbade commented Aug 20, 2015

Hmm there is no 64bit simulator!

@bleege
Copy link
Contributor Author

bleege commented Aug 20, 2015

What about this 64 bit emulator from Google?

@bleege bleege reopened this Aug 20, 2015
@incanus
Copy link
Contributor

incanus commented Sep 2, 2015

Ended up using make android-lib-arm-v8 because something with a 64 in it would make too much sense.

LOL

@incanus
Copy link
Contributor

incanus commented Sep 2, 2015

Currently poking around here in Nexus 9 device vs. Nexus 4 emulator with make android-lib-arm-v8.

Capturing setup from chat with @bleege:

  • Try breaking here to see the app stand up successfully thus far.
  • Something in that routine dies. I typically see problems inside of Inflater.inflate, which speaks to the layout and/or the GL view present in it.
  • This leads @bleege and I to think that the problem is something to do with OpenGL context, view lifecycle, or other runtime aspects and not a build toolchain issue necessarily.

@ljbade
Copy link
Contributor

ljbade commented Sep 3, 2015

Hmm my findings are that it crashes in nativeCreate which is the native side of this

initialize() gets called during the XML inflation logic in the Android framework.

@bleege
Copy link
Contributor Author

bleege commented Sep 3, 2015

Ok, this is going to cause some heads to explode, but I just found the following link on Android's JNI Tips Web site entitled "64-bit Considerations". It reads:

Android is currently expected to run on 32-bit platforms. In theory it could be built for a 64-bit system, but that is not a goal at this time. For the most part this isn't something that you will need to worry about when interacting with native code, but it becomes significant if you plan to store pointers to native structures in integer fields in an object. To support architectures that use 64-bit pointers, you need to stash your native pointers in a long field rather than an int.

So... this means (to me) that we either go back, convert int to long in the entire stack, and cross our fingers that it works OR we remove the arm64-v8a / make android-lib-arm-v8 command and just produce armabi-v7a which we know works based on the testing above on an HTC One M9. My vote would be the latter option (rely on armabi-v7a) with thorough testing on as many 64 bit Android devices that we have (Samsung S6, Nexus 9, HTC One M9). Ideally we'd be able to test on an LG G4 too.

Thoughts?

/cc @incanus @ljbade

@mb12
Copy link

mb12 commented Sep 3, 2015

Please see my comments below to see why creating ARM-V8 specific .so should be possible.

1.) Android NDK includes a sample opengl application. This application creates an ARM V8 specific .so. This uses Android.mk and the built in macro to create the shared library.

If we can we get this application to work with clang++ on ARM64, then we should be able to get mapbox-gl-native working as well.

2.) If you have a rooted 64 bit Android device, you can also check if Google Map includes an ARM-V8 specific .so or not. Or you can just download one from here and verify that it includes an ARM V8 specific .so.

http://www.apkmirror.com/apk/google-inc/maps/

@incanus
Copy link
Contributor

incanus commented Sep 3, 2015

Right @mb12, but I think @bleege's point was that we'd have to audit the GL code base for int vs. long, which is doable in sample apps but a bit more in-depth here.

@mb12
Copy link

mb12 commented Sep 3, 2015

1.) There is absolutely no need to convert int to long in native code base as long as we are using jlong in native code (jni.cpp). long is 4 byte on 32-bit platforms and 8 byte in 64 bit platforms. This is why this data type is used to store pointers in code that is compiled for both 32-bit and 64-bit architectures. This is pretty widely used. This is not java or android specific.

Even mapbox-gl-native code works find on both x86 (32-bit) and x86_64 (64-bit platforms).

2.) Android NDK recommends using Android.mk and built in macros to add architecture specific compile flags. Since we are not using this, its possible that we are not passing the right compiler flags when compiling native code. Trying to get the sample application working with clang++ would help us identify these flags.

Simply eye-balling the build.log for arm-v8 shows that something is amiss. Here is one such example.

When building for armeabi, we add flag "-march=armv5te".
When building for arm-v7, we add flag "-march=armv7-a"
However there is no "-march=arm64-v8a" when building for ARM64. (This may very well not be needed for ARM64. One way to ascertain this is to get the sample application working with clang++ and whatever flags we are using).

@incanus
Copy link
Contributor

incanus commented Sep 3, 2015

Trying to get the sample application working with clang++ would help us identify these flags.

A little out of my depth here, but I will give it a whirl.

@bleege
Copy link
Contributor Author

bleege commented Sep 3, 2015

@mb12 This is all really good to know and I think we could work towards that in time. @incanus is correct about what I'm thinking here.... this is something that we'd have to audit and has a high potential for being rabbit hole time wise (happy to be wrong though!). This plus with what Google is explicitly saying:

In theory it could be built for a 64-bit system, but that is not a goal at this time.

Again, I'm happy to be wrong about all this if it can be done. I'm just trying to be practical as we have 32bit working on 64bit devices, which is (my interpretation of) what Google suggests.

@incanus
Copy link
Contributor

incanus commented Sep 3, 2015

I'm just trying to be practical as we have 32bit working on 64bit devices, which is (my interpretation of) what Google suggests.

Reading around, I am feeling better about this route for android-v0.1.0. If GL runs fine in 32-bit, and there is little to be found around even pure-Java 64-bit apps, I think sticking to merely 32-bit for now is just fine, especially considering that we support plenty of other 32-bit architectures (armv7 and armv7s for iOS and i386 for iOS Simulator, for example) so this exercises the core GL routines and tests (that is, 32-bit Android isn't the only 32-bit we're shipping).

@ljbade
Copy link
Contributor

ljbade commented Sep 3, 2015

We do currently use long for pointers. It's been that way from the very start.

@mb12 If you look in the NDK scripts you will see ARM64 does not use -march instead the ARM64 cross compiler has that setting builtin. It is needed for ARM32 since there can be both ARMv5 and ARMv7 binaries from the same cross compiler.

I vote for 32bit only for 0.1.0 at least. Only issue I can forsee is if a user includes another third party 64bit JNI library alongside Mapbox then it will fail to load libmbgl.so as there is no 64bit version. The fix for this though is for the developer to delete the other 64bit JNI libraries and also convert their application to 32bit only.

We should document this ^ somewhere as it will only show up when testing on a 64bit device.

AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this issue Nov 6, 2015
@tmpsantos tmpsantos self-assigned this Mar 16, 2016
tmpsantos added a commit to mapbox/mason that referenced this issue Mar 16, 2016
We were getting segmentation faults and illegal instructions. My
take is support for aarch64 + C++14 symbols is fresh. I didn't
find anyone else reporting similar issues so I created one on AOP.

AOP issue:
https://code.google.com/p/android/issues/detail?id=204151

Mapbox GL Native issues:
mapbox/mapbox-gl-native#1956
mapbox/mapbox-gl-native#3128
tmpsantos added a commit to mapbox/mason that referenced this issue Mar 16, 2016
We were getting segmentation faults and illegal instructions. My
take is support for aarch64 + C++14 symbols is fresh. I didn't
find anyone else reporting similar issues so I created one on AOP.

AOP issue:
https://code.google.com/p/android/issues/detail?id=204151

Mapbox GL Native issues:
mapbox/mapbox-gl-native#1956
mapbox/mapbox-gl-native#3128
tmpsantos added a commit that referenced this issue Mar 16, 2016
Fixes aarch64 build for android.

Fixes #1956
Fixes #3128
bleege pushed a commit that referenced this issue Mar 17, 2016
Fixes aarch64 build for android.

Fixes #1956
Fixes #3128
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android build
Projects
None yet
Development

No branches or pull requests

7 participants