Skip to content

Commit

Permalink
Option to merge non-asset native libraries as well
Browse files Browse the repository at this point in the history
Summary: The previous merge map logic allowed us to merge non-asset libraries as well, however with the new algorithm we opted for simplicity to avoid non-asset to asset dependencies. As part of StartupSO (https://fb.workplace.com/groups/358819306939597/permalink/471486579006202/), I would like to merge a few non-asset libraries together, as that helps with both performance (load times) and size. Long term, I'll want some better logic in  `merge_sequence.py` to enforce that non-asset libs cannot depend on asset libs, however for now let's have this as a config to allow us experiment with merging non-asset libraries in a controlled way.

Reviewed By: egpast

Differential Revision: D64332722

fbshipit-source-id: 2e3e82a3f87003df5910f90eab0f795cae96ad02
  • Loading branch information
adicatana authored and facebook-github-bot committed Oct 18, 2024
1 parent 7919158 commit 3d0ddcc
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 1 deletion.
3 changes: 3 additions & 0 deletions android/android_binary_native_library_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def get_android_binary_native_library_info(
linkable_nodes_by_platform = {}
native_library_merge_sequence = getattr(ctx.attrs, "native_library_merge_sequence", None)
native_library_merge_map = getattr(ctx.attrs, "native_library_merge_map", None)
native_library_merge_non_asset_libs = getattr(ctx.attrs, "native_library_merge_non_asset_libs", False)
has_native_merging = native_library_merge_sequence or native_library_merge_map
enable_relinker = getattr(ctx.attrs, "enable_relinker", False)

Expand Down Expand Up @@ -211,6 +212,8 @@ def get_android_binary_native_library_info(
mergemap_cmd.add(cmd_args(native_library_merge_input_file, format = "--mergemap-input={}"))
if apk_module_graph_file:
mergemap_cmd.add(cmd_args(apk_module_graph_file, format = "--apk-module-graph={}"))
if native_library_merge_non_asset_libs:
mergemap_cmd.add(cmd_args("--merge-non-asset-libs"))
native_library_merge_dir = ctx.actions.declare_output("merge_sequence_output")
native_library_merge_map = native_library_merge_dir.project("merge.map")
split_groups_map = native_library_merge_dir.project("split_groups.map")
Expand Down
5 changes: 4 additions & 1 deletion android/tools/merge_sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ def get_native_linkables_by_merge_sequence( # noqa: C901
native_library_merge_sequence: list[MergeSequenceGroupSpec],
native_library_merge_sequence_blocklist: list[typing.Pattern],
apk_module_graph: ApkModuleGraph,
native_library_merge_non_asset_libs: bool,
) -> typing.Tuple[dict[Label, NodeData], dict[FinalLibKey, str], FinalLibGraph]:
final_lib_graph = FinalLibGraph()
node_data: dict[Label, NodeData] = {}
Expand All @@ -419,7 +420,7 @@ def get_native_linkables_by_merge_sequence( # noqa: C901

def check_is_excluded(target: Label) -> bool:
node = graph_node_map[target]
if not node.can_be_asset:
if not native_library_merge_non_asset_libs and not node.can_be_asset:
return True

raw_target = node.raw_target
Expand Down Expand Up @@ -714,6 +715,7 @@ def main() -> int: # noqa: C901
parser.add_argument("--mergemap-input", required=True)
parser.add_argument("--apk-module-graph")
parser.add_argument("--output")
parser.add_argument("--merge-non-asset-libs", action="store_true")
args = parser.parse_args()

apk_module_graph = read_apk_module_graph(args.apk_module_graph)
Expand All @@ -732,6 +734,7 @@ def main() -> int: # noqa: C901
mergemap_input.merge_sequence,
mergemap_input.blocklist,
apk_module_graph,
args.merge_non_asset_libs,
)

final_mapping = {}
Expand Down
3 changes: 3 additions & 0 deletions decls/android_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ android_aar = prelude_rule(
"native_library_merge_map": attrs.option(attrs.dict(key = attrs.string(), value = attrs.list(attrs.regex()), sorted = False), default = None),
"native_library_merge_sequence": attrs.option(attrs.list(attrs.any()), default = None),
"native_library_merge_sequence_blocklist": attrs.option(attrs.list(attrs.regex()), default = None),
"native_library_merge_non_asset_libs": attrs.bool(default = False),
"never_mark_as_unused_dependency": attrs.option(attrs.bool(), default = None),
"on_unused_dependencies": attrs.option(attrs.enum(UnusedDependenciesAction), default = None),
"proguard_config": attrs.option(attrs.source(), default = None),
Expand Down Expand Up @@ -236,6 +237,7 @@ android_binary = prelude_rule(
"native_library_merge_map": attrs.option(attrs.dict(key = attrs.string(), value = attrs.list(attrs.regex()), sorted = False), default = None),
"native_library_merge_sequence": attrs.option(attrs.list(attrs.any()), default = None),
"native_library_merge_sequence_blocklist": attrs.option(attrs.list(attrs.regex()), default = None),
"native_library_merge_non_asset_libs": attrs.bool(default = False),
"no_auto_add_overlay_resources": attrs.bool(default = False),
"no_auto_version_resources": attrs.bool(default = False),
"no_dx": attrs.list(attrs.dep(), default = []),
Expand Down Expand Up @@ -470,6 +472,7 @@ android_bundle = prelude_rule(
"native_library_merge_map": attrs.option(attrs.dict(key = attrs.string(), value = attrs.list(attrs.regex()), sorted = False), default = None),
"native_library_merge_sequence": attrs.option(attrs.list(attrs.any()), default = None),
"native_library_merge_sequence_blocklist": attrs.option(attrs.list(attrs.regex()), default = None),
"native_library_merge_non_asset_libs": attrs.bool(default = False),
"no_auto_add_overlay_resources": attrs.bool(default = False),
"no_auto_version_resources": attrs.bool(default = False),
"no_dx": attrs.list(attrs.dep(), default = []),
Expand Down

0 comments on commit 3d0ddcc

Please sign in to comment.