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

Commit

Permalink
Native library merging support
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dreiss authored and Facebook Github Bot 8 committed Aug 10, 2016
1 parent 1445981 commit 7b7bc87
Show file tree
Hide file tree
Showing 15 changed files with 582 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
490 changes: 478 additions & 12 deletions src/com/facebook/buck/android/NativeLibraryMergeEnhancer.java

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions src/com/facebook/buck/cxx/CxxLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,7 @@ public ImmutableSortedSet<BuildRule> getRuntimeDeps() {
.build();
}

public Iterable<Arg> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
return exportedLinkerFlags.apply(cxxPlatform);
}
}
3 changes: 3 additions & 0 deletions src/com/facebook/buck/cxx/PrebuiltCxxLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -461,4 +461,7 @@ public NativeLinkableInput getNativeLinkTargetInput(CxxPlatform cxxPlatform)
});
}

public ImmutableList<String> getExportedLinkerFlags(CxxPlatform cxxPlatform) {
return exportedLinkerFlags.apply(cxxPlatform);
}
}
29 changes: 29 additions & 0 deletions test/com/facebook/buck/android/AndroidBinaryIntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[ndk]
#cpu_abis
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# |
# 2(EF)
#
# Also, C and F each depend on a static and prebuilt library.

cxx_library(
name = 'A',
Expand Down Expand Up @@ -46,6 +47,8 @@ cxx_library(
srcs = ['C.c'],
deps = [
':D',
':static_for_C',
':prebuilt_for_C',
],
)

Expand All @@ -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 = [
],
)
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
void D();
void static_func_C();
void prebuilt_func_C();
void C() {
D();
static_func_C();
prebuilt_func_C();
}
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
void F(){}
void static_func_F();
void prebuilt_func_F();
void F() {
static_func_F();
prebuilt_func_F();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void prebuilt_func_C(){}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void prebuilt_func_F(){}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void static_func_C(){}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void static_func_F(){}

2 comments on commit 7b7bc87

@arpit07531
Copy link

Choose a reason for hiding this comment

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

@dreiss Did you observe any runtime performance improvements and net shared library size gains by merging together libraries?

Ideally, the combined library should load faster as compared to loading all the individual libraries separately, because of reduced load time overhead of resolving undefined symbols.

Also, doing this, the net count of defined and undefined symbols should reduce, thereby reducing the symbols table size and should reduce the net disk size of libraries.

@dreiss
Copy link
Member Author

@dreiss dreiss commented on 7b7bc87 Apr 22, 2019

Choose a reason for hiding this comment

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

Size was reduced. We were not able to measure any statistically significant change in performance.

Please sign in to comment.