Add support for NDK21 #6460

cmelchior · 2019-03-12T14:55:14Z

This PR bumps the NDK support from r10e to r21

Closes #6408
Closes #3506
Closes #3623

It has the following impact:

minSdk is bumped from 9 to 16 (This will impact ~1% of current users)
MIPS support is dropped. Google already did this, and to our knowledge, no users would be affected.
Dropping MIPS will reduce universal APK size by ~1.5MB. People using App Split or App Bundle will not be affected.
Native code is now compiled with Clang instead of GCC.
OpenSSL is upgraded from 1.0.2 to 1.1.1.

TODO

Merge NDK 18 support in Object Store and redirect OS submodule.
Our prebuilt OpenSSL needs to be built without including ARM assembler code. It breaks Android.
OpenSSL was apparently built with minSDK 21. This was mostly done to include 64 bit support which was added in 21. But 32 bit builds should be done using minSdk 16.
Test size differences for both base and objectServer builds.
Performance benchmarks
Fix Core compiler flags
Fix Sync compiler flags
Fix OpenSSL compiler flags

See https://github.com/realm/openssl-android-termux

Fix merge from 9c0963 to next-major

# Conflicts: # CHANGELOG.md # version.txt

Fix merge from efb60f to next-major

cmelchior · 2020-02-21T08:18:06Z

Comparing next-major built with NDK10 with this branch

NDK 21 - Default Linker (not LLD)

Analyzing Base...
Method count: 3490
AAR size: 8.161.817
3871584 ./unzippedBase/jni/armeabi-v7a/librealm-jni.so
5702688 ./unzippedBase/jni/x86/librealm-jni.so
5529928 ./unzippedBase/jni/arm64-v8a/librealm-jni.so
6120032 ./unzippedBase/jni/x86_64/librealm-jni.so

Analyzing ObjectServer...
Method count: 4742
AAR size: 11.280.391
5119940 ./unzippedObjectServer/jni/armeabi-v7a/librealm-jni.so
7590088 ./unzippedObjectServer/jni/x86/librealm-jni.so
7318024 ./unzippedObjectServer/jni/arm64-v8a/librealm-jni.so
8055616 ./unzippedObjectServer/jni/x86_64/librealm-jni.so

NDK r10e

Analyzing Base...
Method count: 3490
AAR size: 6.882.507
3118736 ./unzippedBase/jni/armeabi-v7a/librealm-jni.so
5040132 ./unzippedBase/jni/x86/librealm-jni.so
4568464 ./unzippedBase/jni/arm64-v8a/librealm-jni.so
4897488 ./unzippedBase/jni/x86_64/librealm-jni.so

Analyzing ObjectServer...
Method count: 4742
AAR size: 10.202.000
4521908 ./unzippedObjectServer/jni/armeabi-v7a/librealm-jni.so
7018692 ./unzippedObjectServer/jni/x86/librealm-jni.so
6432032 ./unzippedObjectServer/jni/arm64-v8a/librealm-jni.so
6816144 ./unzippedObjectServer/jni/x86_64/librealm-jni.so

Flag notes

Adding -fno-stack-protector saves about 60Kb. Worth thinking about.

cmelchior · 2020-02-21T14:54:20Z

@nhachicha this PR is now ready for review.
Note there are two outstanding issues with an issue tracking them here: #6754

But I don't think they should stop us from merging this PR as NDK21 support is required to adopt any of the new datatypes being developed in Core/ObjectStore.

nhachicha

minor things 👍

nhachicha · 2020-02-24T12:38:14Z

realm/realm-library/src/main/cpp/CMakeLists.txt

-set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNDEBUG")
-#-ggdb doesn't play well with -flto
+set(CMAKE_CXX_FLAGS_RELEASE "-DNDEBUG -Oz")
+# -ggdb doesn't play well with -flto
 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -ggdb -Og")


similarly, should we suppress optimisation for glldb as well?

nhachicha · 2020-02-24T12:55:50Z

realm/realm-library/src/main/cpp/io_realm_SyncSession.cpp

@@ -157,8 +157,8 @@ JNIEXPORT jboolean JNICALL Java_io_realm_SyncSession_nativeWaitForDownloadComple
            static JavaClass java_sync_session_class(env, "io/realm/SyncSession");
            static JavaMethod java_notify_result_method(env, java_sync_session_class, "notifyAllChangesSent",
                                                        "(ILjava/lang/Long;Ljava/lang/String;)V");
-            JavaGlobalRef java_session_object_ref(env, session_object);


btw JavaGlobalRef has move assignment operator so

session->wait_for_download_completion([java_session_object_ref=std::move(java_session_object_ref), callback_id](std::error_code error) {

should work I think, since the moved java_session_object_ref will be deleted inside the callback scope

I'm getting the following error no matter what I do. Even using your approach. I'm suspecting it is somehow related to how the JavaGlobalRef class is defined, but no idea how to fix it

/Users/cm/Realm/realm-java/realm/realm-library/src/main/cpp/io_realm_SyncSession.cpp:194:49: note: candidate constructor (the implicit move constructor) not viable: 1st argument ('const (lambda at /Users/cm/Realm/realm-java/realm/realm-library/src/main/cpp/io_realm_SyncSession.cpp:194:49)') would lose const qualifier

nhachicha · 2020-02-24T12:57:39Z

realm/realm-library/src/main/cpp/io_realm_internal_OsRealmConfig.cpp

@@ -291,7 +291,7 @@ JNIEXPORT jstring JNICALL Java_io_realm_internal_OsRealmConfig_nativeCreateAndSe
                    case ECONNABORTED: error_code = 113; break;
                    default:
                        /* Do nothing */
-                        error_code = error_code;
+                        (void)0;


you could use REALM_UNREACHABLE(); instead

It isn't unreachable, you might end up here, but if there is no code here it will trigger a warning which breaks compilation

nhachicha · 2020-02-24T12:58:04Z

realm/realm-library/src/main/cpp/io_realm_internal_OsRealmConfig.cpp

@@ -300,7 +300,7 @@ JNIEXPORT jstring JNICALL Java_io_realm_internal_OsRealmConfig_nativeCreateAndSe
                    case util::MiscExtErrors::delim_not_found: error_code = 3; break;
                    default:
                        /* Do nothing */
-                        error_code = error_code;
+                        (void)0;


nhachicha · 2020-02-24T13:54:16Z

realm/realm-library/src/main/java/io/realm/internal/sync/PermissionHelper.java

@@ -38,6 +38,9 @@
     * @return
     */
    public static Permission findOrCreatePermissionForRole(RealmObject container, RealmList<Permission> permissions, String roleName) {
+        if (roleName == null) {


Suggested change

if (roleName == null) {

if (Util.isEmptyString(roleName)) {

.../realm-library/src/syncIntegrationTest/java/io/realm/objectserver/ProgressListenerTests.java

nhachicha · 2020-02-24T14:03:49Z

tools/analyze_realm_metrics.sh

+# To run it:
+#   1. Make sure that $D8 is defined in your environment, e.g. `D8="$ANDROID_SDK_ROOT/build-tools/29.0.2/d8"`
+#   2. Make sure that you have built the library artifacts using `./gradlew assembleRelease` from the realm folder.
+#   3. Run the script: `> sh ./analyze_realm_metrics.sh`


BTW, running this from tools or <root> or <root/realm> always report cat: ./unzippedBase/classes.dex: No such file or directory

Analyzing Base... AAR size: 2476366 cat: ./unzippedBase/classes.dex: No such file or directory 5641248 ./unzippedBase/jni/x86/librealm-jni.so Analyzing ObjectServer... AAR size: 3476405 cat: ./unzippedObjectServer/classes.dex: No such file or directory

realm-ci and others added 30 commits March 14, 2018 16:43

Merge 383208f into next-major

1203872

Merge fbeb212 into next-major

58ffc86

Merge 5497ac9 into next-major

7f60f45

Prepare for next major version

002b77a

Merge 087417e into next-major

81e1e4b

Merge ad302da into next-major

65bc651

Merge 5a9dd47 into next-major

fc7875e

Merge fbfdb41 into next-major

5ade758

Merge f243b8f into next-major

602a27d

Merge ddbebcb into next-major

9c9cf51

Merge 76fdc09 into next-major

5ef0646

Merge 3ee7fa2 into next-major

83472df

Merge c124469 into next-major

5b25676

Merge abd2f9b into next-major

8a7ab2b

Merge b7b33ce into next-major

e86a46c

Merge 8d37b06 into next-major

6765c46

Merge 6c6b625 into next-major

c7e2d62

Merge e305a58 into next-major

b7caa69

Merge branch 'next-major' into merge-9c0963-to-next-major

e52b288

Merge pull request #5886 from realm/merge-9c0963-to-next-major

c2142d8

Fix merge from 9c0963 to next-major

Merge f755bbc into next-major

22246f6

Merge ad88afe into next-major

c0c1792

Query-based sync are now the default mode for Sync (#5893)

475bba0

Use query-based sync terminology in all docs.

68a95c5

Merge 8162396 into next-major

4744e22

Merge branch 'next-major' into merge-efb60f-to-next-major

4e3579e

# Conflicts: # CHANGELOG.md # version.txt

Merge pull request #5920 from realm/merge-efb60f-to-next-major

6e52157

Fix merge from efb60f to next-major

Merge 7282c27 into next-major

57ed208

Merge 3529e04 into next-major

de4e17f

Merge 3f14e88 into next-major

7dadcfe

cmelchior added 13 commits February 13, 2020 19:44

Update OS

5273516

Disable unevaluated-expression

3a42f0b

Disable more checks

e09c855

Don't use JavaGlobalRef in lambda capture

d1832a0

Reduce CMake version

ef3862c

Disable more checks

a017c05

Remove test

206427f

Fix test

522225c

Cleanup. Disable broken test. Fix JSON tests

465fe9b

Disable correct test

829f306

Temp disable all Progress listener tests f

4225b3d

Add helper script for finding size of library

f156998

Also print AAR size

6f05d27

cmelchior added 4 commits February 21, 2020 12:21

Add debug for failing tests

36da844

Has notifications gotten slower?

a9a1402

Merge mistake

67be8ea

Cleanup

6c73e8a

cmelchior mentioned this pull request Feb 21, 2020

NDK21: Fix ProgressListeners + Client Resync #6754

Closed

cmelchior marked this pull request as ready for review February 21, 2020 14:53

cmelchior assigned nhachicha Feb 21, 2020

nhachicha approved these changes Feb 24, 2020

View reviewed changes

nhachicha assigned cmelchior and unassigned nhachicha Feb 24, 2020

PR feedback

cacac37

cmelchior mentioned this pull request Feb 24, 2020

NDK21: Find a way to use JavaGlobalRef in progress listeners #6755

Closed

cmelchior merged commit a0aa3b9 into next-major Feb 24, 2020

cmelchior deleted the cm/ndk18 branch February 24, 2020 22:11

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for NDK21 #6460

Add support for NDK21 #6460

cmelchior commented Mar 12, 2019 •

edited

Loading

cmelchior commented Feb 21, 2020 •

edited

Loading

cmelchior commented Feb 21, 2020 •

edited

Loading

nhachicha left a comment

nhachicha Feb 24, 2020

nhachicha Feb 24, 2020

cmelchior Feb 24, 2020

nhachicha Feb 24, 2020

cmelchior Feb 24, 2020

nhachicha Feb 24, 2020

nhachicha Feb 24, 2020

nhachicha Feb 24, 2020

Add support for NDK21 #6460

Add support for NDK21 #6460

Conversation

cmelchior commented Mar 12, 2019 • edited Loading

cmelchior commented Feb 21, 2020 • edited Loading

NDK 21 - Default Linker (not LLD)

NDK r10e

Flag notes

cmelchior commented Feb 21, 2020 • edited Loading

nhachicha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmelchior commented Mar 12, 2019 •

edited

Loading

cmelchior commented Feb 21, 2020 •

edited

Loading

cmelchior commented Feb 21, 2020 •

edited

Loading