Native library merging support

dreiss · Facebook Github Bot 8 · commit 7b7bc87e124e · 2016-08-09T17:20:11.000-07:00
Summary:
Older versions of Android have a limit on how many DSOs they can load
into one process.  To work around this limit, it can be helpful to merge
multiple libraries together based on a per-app configuration.

This completes NativeLibraryMergeEnhancer to actually perform the
merging.

Test Plan:
Added integration tests.
Tested a few real merges with Facebook for Android.

Reviewed By: andrewjcg

fbshipit-source-id: 7511741
diff --git a/src/com/facebook/buck/android/AndroidNativeLibsPackageableGraphEnhancer.java b/src/com/facebook/buck/android/AndroidNativeLibsPackageableGraphEnhancer.java
@@ -117,7 +117,7 @@ public Optional<CopyNativeLibraries> getCopyNativeLibraries(
     ImmutableList<NativeLinkable> nativeLinkablesAssets =
         packageableCollection.getNativeLinkablesAssets();
 
-    if (nativeLibraryMergeMap.isPresent()) {
+    if (nativeLibraryMergeMap.isPresent() && !nativeLibraryMergeMap.get().isEmpty()) {
       NativeLibraryMergeEnhancementResult enhancement = NativeLibraryMergeEnhancer.enhance(
           cxxBuckConfig,
           ruleResolver,
diff --git a/src/com/facebook/buck/android/NativeLibraryMergeEnhancer.java b/src/com/facebook/buck/android/NativeLibraryMergeEnhancer.java
diff --git a/src/com/facebook/buck/cxx/CxxLibrary.java b/src/com/facebook/buck/cxx/CxxLibrary.java
@@ -367,4 +367,7 @@ public ImmutableSortedSet<BuildRule> getRuntimeDeps() {
         .build();
   }
 
+  public Iterable<Arg> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
+    return exportedLinkerFlags.apply(cxxPlatform);
+  }
 }
diff --git a/src/com/facebook/buck/cxx/PrebuiltCxxLibrary.java b/src/com/facebook/buck/cxx/PrebuiltCxxLibrary.java
@@ -461,4 +461,7 @@ public NativeLinkableInput getNativeLinkTargetInput(CxxPlatform cxxPlatform)
         });
   }
 
+  public ImmutableList<String> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
+    return exportedLinkerFlags.apply(cxxPlatform);
+  }
 }
diff --git a/test/com/facebook/buck/android/AndroidBinaryIntegrationTest.java b/test/com/facebook/buck/android/AndroidBinaryIntegrationTest.java
@@ -19,6 +19,7 @@
 import static com.facebook.buck.testutil.RegexMatcher.containsRegex;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -361,14 +362,42 @@ public void testNativeLibraryMerging() throws IOException, InterruptedException
     SymbolGetter syms = new SymbolGetter(tmpDir, platform.getObjdump(), pathResolver);
     SymbolsAndDtNeeded info;
 
+    workspace.replaceFileContents(
+        ".buckconfig",
+        "#cpu_abis",
+        "cpu_abis = x86");
     Path apkPath = workspace.buildAndReturnOutput("//apps/sample:app_with_merged_libs");
 
+    ZipInspector zipInspector = new ZipInspector(apkPath);
+    zipInspector.assertFileDoesNotExist("lib/x86/lib1a.so");
+    zipInspector.assertFileDoesNotExist("lib/x86/lib1b.so");
+    zipInspector.assertFileDoesNotExist("lib/x86/lib2e.so");
+    zipInspector.assertFileDoesNotExist("lib/x86/lib2f.so");
+
+    info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/lib1.so");
+    assertThat(info.symbols.global, Matchers.hasItem("A"));
+    assertThat(info.symbols.global, Matchers.hasItem("B"));
+    assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_C.so"));
+    assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_D.so"));
+    assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_B.so")));
+
     info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/libnative_merge_C.so");
     assertThat(info.symbols.global, Matchers.hasItem("C"));
+    assertThat(info.symbols.global, Matchers.hasItem("static_func_C"));
     assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_D.so"));
+    assertThat(info.dtNeeded, Matchers.hasItem("libprebuilt_for_C.so"));
 
     info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/libnative_merge_D.so");
     assertThat(info.symbols.global, Matchers.hasItem("D"));
+    assertThat(info.dtNeeded, Matchers.hasItem("lib2.so"));
+    assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_E.so")));
+    assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_F.so")));
+
+    info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/lib2.so");
+    assertThat(info.symbols.global, Matchers.hasItem("E"));
+    assertThat(info.symbols.global, Matchers.hasItem("F"));
+    assertThat(info.symbols.global, Matchers.hasItem("static_func_F"));
+    assertThat(info.dtNeeded, Matchers.hasItem("libprebuilt_for_F.so"));
   }
 
   @Test
diff --git a/test/com/facebook/buck/android/testdata/android_project/.buckconfig b/test/com/facebook/buck/android/testdata/android_project/.buckconfig
@@ -0,0 +1,2 @@
+[ndk]
+  #cpu_abis
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/BUCK.fixture b/test/com/facebook/buck/android/testdata/android_project/native/merge/BUCK.fixture
@@ -18,6 +18,7 @@
 #    |
 #    2(EF)
 #
+# Also, C and F each depend on a static and prebuilt library.
 
 cxx_library(
   name = 'A',
@@ -46,6 +47,8 @@ cxx_library(
   srcs = ['C.c'],
   deps = [
     ':D',
+    ':static_for_C',
+    ':prebuilt_for_C',
   ],
 )
 
@@ -71,6 +74,55 @@ cxx_library(
   srcs = ['F.c'],
   soname = 'lib2f.so',
   deps = [
+    ':static_for_F',
+    ':prebuilt_for_F',
   ],
 )
 
+cxx_library(
+  name = 'static_for_C',
+  srcs = ['static_for_C.c'],
+  force_static = True,
+  deps = [
+  ],
+)
+
+cxx_library(
+  name = 'static_for_F',
+  srcs = ['static_for_F.c'],
+  force_static = True,
+  deps = [
+  ],
+)
+
+prebuilt_cxx_library(
+  name = 'prebuilt_for_C',
+  lib_dir = 'prebuilt_for_C/$(platform)',
+  deps = [
+  ],
+)
+
+prebuilt_cxx_library(
+  name = 'prebuilt_for_F',
+  lib_dir = 'prebuilt_for_F/$(platform)',
+  deps = [
+  ],
+)
+
+# Built manually to create .so file for prebuilt_for_C
+cxx_library(
+  name = 'prebuilt_for_C_src',
+  soname = 'libprebuilt_for_C.so',
+  srcs = ['prebuilt_for_C.c'],
+  deps = [
+  ],
+)
+
+# Built manually to create .so file for prebuilt_for_F
+cxx_library(
+  name = 'prebuilt_for_F_src',
+  soname = 'libprebuilt_for_F.so',
+  srcs = ['prebuilt_for_F.c'],
+  deps = [
+  ],
+)
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/C.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/C.c
@@ -1,4 +1,8 @@
 void D();
+void static_func_C();
+void prebuilt_func_C();
 void C() {
   D();
+  static_func_C();
+  prebuilt_func_C();
 }
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/F.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/F.c
@@ -1 +1,6 @@
-void F(){}
+void static_func_F();
+void prebuilt_func_F();
+void F() {
+  static_func_F();
+  prebuilt_func_F();
+}
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_C.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_C.c
@@ -0,0 +1 @@
+void prebuilt_func_C(){}
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_C/android-x86/libprebuilt_for_C.so b/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_C/android-x86/libprebuilt_for_C.so
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_F.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_F.c
@@ -0,0 +1 @@
+void prebuilt_func_F(){}
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_F/android-x86/libprebuilt_for_F.so b/test/com/facebook/buck/android/testdata/android_project/native/merge/prebuilt_for_F/android-x86/libprebuilt_for_F.so
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/static_for_C.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/static_for_C.c
@@ -0,0 +1 @@
+void static_func_C(){}
diff --git a/test/com/facebook/buck/android/testdata/android_project/native/merge/static_for_F.c b/test/com/facebook/buck/android/testdata/android_project/native/merge/static_for_F.c
@@ -0,0 +1 @@
+void static_func_F(){}
