Don't recompute LinkGroupInfo

Summary: Instead of computing `LinkGroupInfo` twice, extract the map definition and defer to expensive target-to-group mapping computation until after we have all the auto generated group inormation.

Reviewed By: JakobDegen

Differential Revision: D69619079

fbshipit-source-id: 7b47a0605e39581ac2fb298182bb8dabbc6ec51b

prelude/cxx/cxx.bzl

@@ -233,10 +233,10 @@ def _only_shared_mappings(group: Group) -> bool:
             return False
     return True
 
-def create_shared_lib_link_group_specs(ctx: AnalysisContext, link_group_info: LinkGroupInfo) -> list[LinkGroupLibSpec]:
+def create_shared_lib_link_group_specs(ctx: AnalysisContext, link_group_definitions: list[Group]) -> list[LinkGroupLibSpec]:
     specs = []
     linker_info = get_cxx_toolchain_info(ctx).linker_info
-    for group in link_group_info.groups.values():
+    for group in link_group_definitions:
         if group.name in (MATCH_ALL_LABEL, NO_MATCH_LABEL):
             continue
 
@@ -256,7 +256,7 @@ def create_shared_lib_link_group_specs(ctx: AnalysisContext, link_group_info: Li
 def get_auto_link_group_specs(ctx: AnalysisContext, link_group_info: [LinkGroupInfo, None]) -> [list[LinkGroupLibSpec], None]:
     if link_group_info == None or not ctx.attrs.auto_link_groups:
         return None
-    return create_shared_lib_link_group_specs(ctx, link_group_info)
+    return create_shared_lib_link_group_specs(ctx, link_group_info.groups.values())
 
 def cxx_binary_impl(ctx: AnalysisContext) -> list[Provider]:
     link_group_info = get_link_group_info(ctx, filter_and_map_idx(LinkableGraph, cxx_attr_deps(ctx)))

prelude/cxx/groups.bzl

@@ -89,8 +89,14 @@ def parse_groups_definitions(
         # callers to have different top-level types for the `root`s.
         parse_root: typing.Callable = lambda d: d) -> list[Group]:
     groups = []
+    group_names = set()
     for map_entry in map:
         name = map_entry[0]
+
+        # Dedup the link group specs, take the deinition from the first definition
+        if name in group_names:
+            continue
+        group_names.add(name)
         mappings = map_entry[1]
         attrs = (map_entry[2] or {}) if len(map_entry) > 2 else {}
 

prelude/cxx/user/unconfigured_link_group_map.bzl

@@ -17,10 +17,14 @@ load(
 load("@prelude//user:rule_spec.bzl", "RuleRegistrationSpec")
 
 def _impl(ctx: AnalysisContext) -> list[Provider]:
+    if ctx.attrs.link_group_map:
+        definitions = parse_groups_definitions(ctx.attrs.link_group_map)
+    else:
+        definitions = []
     return [
         DefaultInfo(),
         LinkGroupDefinitions(
-            definitions = parse_groups_definitions(ctx.attrs.link_group_map),
+            definitions = definitions,
         ),
     ]
 

prelude/python/linking/native.bzl

@@ -29,6 +29,7 @@ load(
 )
 load(
     "@prelude//cxx:link_groups_types.bzl",
+    "LinkGroupDefinitions",
     "LinkGroupInfo",  # @unused Used as a type
 )
 load("@prelude//cxx:linker.bzl", "get_rpath_origin")
@@ -161,60 +162,59 @@ def _get_link_group_info(
         link_deps: list[LinkableProviders],
         libs: list[LinkableProviders],
         extensions: dict[str, LinkableProviders],
-        shared_only_libs: list[LinkableProviders]) -> (LinkGroupInfo, list[LinkGroupLibSpec]):
+        shared_only_libs: list[LinkableProviders]) -> ([LinkGroupInfo, None], list[LinkGroupLibSpec]):
     """
     Return the `LinkGroupInfo` and link group lib specs to use for this binary.
     This will handle parsing the various user-specific parameters and automatic
     link group lib spec generation for dlopen-enabled native libraries and,
     eventually, extensions.
     """
 
-    link_group_info = get_link_group_info(ctx, [d.linkable_graph for d in link_deps])
-    link_group_specs = []
+    link_group_map = ctx.attrs.link_group_map
+    definitions = []
 
-    # Add link group specs from user-provided link group info.
-    if link_group_info != None:
-        link_group_specs.extend(create_shared_lib_link_group_specs(ctx, link_group_info))
+    if isinstance(link_group_map, Dependency):
+        if LinkGroupDefinitions in link_group_map:
+            definitions = link_group_map[LinkGroupDefinitions].definitions
+
+    if not definitions:
+        link_group_info = get_link_group_info(ctx, [d.linkable_graph for d in link_deps])
+
+        # Add link group specs from user-provided link group info.
+        if link_group_info != None:
+            definitions = link_group_info.groups.values()
 
     # Add link group specs from dlopenable C++ libraries.
     root_specs = _get_root_link_group_specs(libs, extensions)
 
-    # Add link group specs for shared-only libs, which makes sure we link
-    # against them dynamically.
-    shared_groups = _get_shared_only_groups(shared_only_libs)
-
-    # (Re-)build the link group info
-    if root_specs or shared_groups or link_group_info == None:
-        # We prepend the dlopen roots, so that they take precedence over
-        # user-specific ones.
-        link_group_specs = root_specs + link_group_specs
-
-        # Regenerate the new `LinkGroupInfo` with the new link group lib
-        # groups.
-        linkable_graph = LinkableGraph(
-            #label = ctx.label,
-            nodes = ctx.actions.tset(
-                LinkableGraphTSet,
-                children = (
-                    [d.linkable_graph.nodes for d in link_deps] +
-                    [d.linkable_graph.nodes for d in libs] +
-                    [d.linkable_graph.nodes for d in extensions.values()] +
-                    [d.linkable_graph.nodes for d in shared_only_libs]
-                ),
+    # We prepend the dlopen roots, so that they take precedence over
+    # user-specific ones.
+    link_group_specs = root_specs + create_shared_lib_link_group_specs(ctx, definitions)
+
+    # We add the auto-generated root specs first so it takes precedence (as
+    # we really rely on this for things to work), followed by shared-only libs
+    # to make sure we link against them dynamically. Add user-defined mappings
+    # last.
+    link_groups = [s.group for s in root_specs] + _get_shared_only_groups(shared_only_libs) + definitions
+
+    linkable_graph = LinkableGraph(
+        #label = ctx.label,
+        nodes = ctx.actions.tset(
+            LinkableGraphTSet,
+            children = (
+                [d.linkable_graph.nodes for d in link_deps] +
+                [d.linkable_graph.nodes for d in libs] +
+                [d.linkable_graph.nodes for d in extensions.values()] +
+                [d.linkable_graph.nodes for d in shared_only_libs]
             ),
-        )
-
-        # We add user-defined mappings last, so that our auto-generated
-        # ones get precedence (as we rely on this for things to work).
-        link_groups = [s.group for s in root_specs] + shared_groups
-        if link_group_info != None:
-            link_groups += link_group_info.groups.values()
+        ),
+    )
 
-        link_group_info = build_link_group_info(
-            graph = linkable_graph,
-            groups = link_groups,
-            min_node_count = ctx.attrs.link_group_min_binary_node_count,
-        )
+    link_group_info = build_link_group_info(
+        graph = linkable_graph,
+        groups = link_groups,
+        min_node_count = ctx.attrs.link_group_min_binary_node_count,
+    )
 
     return (link_group_info, link_group_specs)