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

Commit 7b7bc87

Browse files
dreissFacebook Github Bot 8
authored andcommitted
Native library merging support
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
1 parent 1445981 commit 7b7bc87

File tree

15 files changed

+582
-14
lines changed

15 files changed

+582
-14
lines changed

src/com/facebook/buck/android/AndroidNativeLibsPackageableGraphEnhancer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public Optional<CopyNativeLibraries> getCopyNativeLibraries(
117117
ImmutableList<NativeLinkable> nativeLinkablesAssets =
118118
packageableCollection.getNativeLinkablesAssets();
119119

120-
if (nativeLibraryMergeMap.isPresent()) {
120+
if (nativeLibraryMergeMap.isPresent() && !nativeLibraryMergeMap.get().isEmpty()) {
121121
NativeLibraryMergeEnhancementResult enhancement = NativeLibraryMergeEnhancer.enhance(
122122
cxxBuckConfig,
123123
ruleResolver,

src/com/facebook/buck/android/NativeLibraryMergeEnhancer.java

Lines changed: 478 additions & 12 deletions
Large diffs are not rendered by default.

src/com/facebook/buck/cxx/CxxLibrary.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,4 +367,7 @@ public ImmutableSortedSet<BuildRule> getRuntimeDeps() {
367367
.build();
368368
}
369369

370+
public Iterable<Arg> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
371+
return exportedLinkerFlags.apply(cxxPlatform);
372+
}
370373
}

src/com/facebook/buck/cxx/PrebuiltCxxLibrary.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,4 +461,7 @@ public NativeLinkableInput getNativeLinkTargetInput(CxxPlatform cxxPlatform)
461461
});
462462
}
463463

464+
public ImmutableList<String> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
465+
return exportedLinkerFlags.apply(cxxPlatform);
466+
}
464467
}

test/com/facebook/buck/android/AndroidBinaryIntegrationTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.facebook.buck.testutil.RegexMatcher.containsRegex;
2020
import static java.nio.charset.StandardCharsets.UTF_8;
2121
import static org.hamcrest.CoreMatchers.containsString;
22+
import static org.hamcrest.CoreMatchers.not;
2223
import static org.junit.Assert.assertEquals;
2324
import static org.junit.Assert.assertFalse;
2425
import static org.junit.Assert.assertNotNull;
@@ -361,14 +362,42 @@ public void testNativeLibraryMerging() throws IOException, InterruptedException
361362
SymbolGetter syms = new SymbolGetter(tmpDir, platform.getObjdump(), pathResolver);
362363
SymbolsAndDtNeeded info;
363364

365+
workspace.replaceFileContents(
366+
".buckconfig",
367+
"#cpu_abis",
368+
"cpu_abis = x86");
364369
Path apkPath = workspace.buildAndReturnOutput("//apps/sample:app_with_merged_libs");
365370

371+
ZipInspector zipInspector = new ZipInspector(apkPath);
372+
zipInspector.assertFileDoesNotExist("lib/x86/lib1a.so");
373+
zipInspector.assertFileDoesNotExist("lib/x86/lib1b.so");
374+
zipInspector.assertFileDoesNotExist("lib/x86/lib2e.so");
375+
zipInspector.assertFileDoesNotExist("lib/x86/lib2f.so");
376+
377+
info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/lib1.so");
378+
assertThat(info.symbols.global, Matchers.hasItem("A"));
379+
assertThat(info.symbols.global, Matchers.hasItem("B"));
380+
assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_C.so"));
381+
assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_D.so"));
382+
assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_B.so")));
383+
366384
info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/libnative_merge_C.so");
367385
assertThat(info.symbols.global, Matchers.hasItem("C"));
386+
assertThat(info.symbols.global, Matchers.hasItem("static_func_C"));
368387
assertThat(info.dtNeeded, Matchers.hasItem("libnative_merge_D.so"));
388+
assertThat(info.dtNeeded, Matchers.hasItem("libprebuilt_for_C.so"));
369389

370390
info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/libnative_merge_D.so");
371391
assertThat(info.symbols.global, Matchers.hasItem("D"));
392+
assertThat(info.dtNeeded, Matchers.hasItem("lib2.so"));
393+
assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_E.so")));
394+
assertThat(info.dtNeeded, not(Matchers.hasItem("libnative_merge_F.so")));
395+
396+
info = syms.getSymbolsAndDtNeeded(apkPath, "lib/x86/lib2.so");
397+
assertThat(info.symbols.global, Matchers.hasItem("E"));
398+
assertThat(info.symbols.global, Matchers.hasItem("F"));
399+
assertThat(info.symbols.global, Matchers.hasItem("static_func_F"));
400+
assertThat(info.dtNeeded, Matchers.hasItem("libprebuilt_for_F.so"));
372401
}
373402

374403
@Test
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[ndk]
2+
#cpu_abis

test/com/facebook/buck/android/testdata/android_project/native/merge/BUCK.fixture

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
# |
1919
# 2(EF)
2020
#
21+
# Also, C and F each depend on a static and prebuilt library.
2122

2223
cxx_library(
2324
name = 'A',
@@ -46,6 +47,8 @@ cxx_library(
4647
srcs = ['C.c'],
4748
deps = [
4849
':D',
50+
':static_for_C',
51+
':prebuilt_for_C',
4952
],
5053
)
5154

@@ -71,6 +74,55 @@ cxx_library(
7174
srcs = ['F.c'],
7275
soname = 'lib2f.so',
7376
deps = [
77+
':static_for_F',
78+
':prebuilt_for_F',
7479
],
7580
)
7681

82+
cxx_library(
83+
name = 'static_for_C',
84+
srcs = ['static_for_C.c'],
85+
force_static = True,
86+
deps = [
87+
],
88+
)
89+
90+
cxx_library(
91+
name = 'static_for_F',
92+
srcs = ['static_for_F.c'],
93+
force_static = True,
94+
deps = [
95+
],
96+
)
97+
98+
prebuilt_cxx_library(
99+
name = 'prebuilt_for_C',
100+
lib_dir = 'prebuilt_for_C/$(platform)',
101+
deps = [
102+
],
103+
)
104+
105+
prebuilt_cxx_library(
106+
name = 'prebuilt_for_F',
107+
lib_dir = 'prebuilt_for_F/$(platform)',
108+
deps = [
109+
],
110+
)
111+
112+
# Built manually to create .so file for prebuilt_for_C
113+
cxx_library(
114+
name = 'prebuilt_for_C_src',
115+
soname = 'libprebuilt_for_C.so',
116+
srcs = ['prebuilt_for_C.c'],
117+
deps = [
118+
],
119+
)
120+
121+
# Built manually to create .so file for prebuilt_for_F
122+
cxx_library(
123+
name = 'prebuilt_for_F_src',
124+
soname = 'libprebuilt_for_F.so',
125+
srcs = ['prebuilt_for_F.c'],
126+
deps = [
127+
],
128+
)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
void D();
2+
void static_func_C();
3+
void prebuilt_func_C();
24
void C() {
35
D();
6+
static_func_C();
7+
prebuilt_func_C();
48
}
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
void F(){}
1+
void static_func_F();
2+
void prebuilt_func_F();
3+
void F() {
4+
static_func_F();
5+
prebuilt_func_F();
6+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void prebuilt_func_C(){}

0 commit comments

Comments
 (0)