perf(dependency_graph): optimize remove_dependency to avoid full-graph scans

- Replace recursive orphan removal with iterative BFS
- Use direct parent/child references instead of scanning all nodes
- Add tests for diamond cascade and cross-subtree survival

Signed-off-by: Josef Skladanka <jskladan@redhat.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>

Author: jskladan

src/fromager/dependency_graph.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import collections
 import dataclasses
 import graphlib
 import json
@@ -349,7 +350,7 @@ def remove_dependency(
         """Remove a dependency node and any orphaned descendants from the graph.
 
         Removes the node and all edges pointing to it. Child nodes that have
-        no remaining parents after removal are recursively removed as well.
+        no remaining parents after removal are iteratively removed as well.
 
         Args:
             req_name: Canonical name of the package
@@ -360,36 +361,50 @@ def remove_dependency(
             logger.debug(f"Cannot remove {key} - not in graph")
             return
 
-        logger.debug(f"Removing failed dependency {key} from graph")
+        queue: collections.deque[str] = collections.deque([key])
 
-        deleted_node = self.nodes[key]
-
-        children = [edge.destination_node for edge in deleted_node.children]
-
-        # Clean up back-references (parents) in nodes that were children of the removed node
-        for child_edge in deleted_node.children:
-            child_node = child_edge.destination_node
-            filtered_parents = [
-                edge for edge in child_node.parents if edge.destination_node.key != key
-            ]
-            child_node.parents.clear()
-            child_node.parents.extend(filtered_parents)
-
-        # Remove the node itself
-        del self.nodes[key]
-
-        # Remove forward edges from any node whose children pointed to the removed node
-        for node in self.nodes.values():
-            filtered_children = [
-                edge for edge in node.children if edge.destination_node.key != key
-            ]
-            node.children.clear()
-            node.children.extend(filtered_children)
+        # BFS over orphaned descendants
+        while queue:
+            key = queue.popleft()
+            # This is a defensive check, should not happen in normal operation
+            if key not in self.nodes:
+                logger.debug(f"Cannot remove {key} - not in graph")
+                continue
 
-        # Recursively remove children that have no remaining parents
-        for child in children:
-            if child.key != ROOT and child.key in self.nodes and not child.parents:
-                self.remove_dependency(child.canonicalized_name, child.version)
+            logger.debug(f"Removing failed dependency {key} from graph")
+
+            deleted_node = self.nodes[key]
+
+            # Remove references to this node from its direct children
+            children = []
+            for child_edge in deleted_node.children:
+                child_node = child_edge.destination_node
+                filtered_parents = [
+                    edge
+                    for edge in child_node.parents
+                    if edge.destination_node.key != key
+                ]
+                child_node.parents.clear()
+                child_node.parents.extend(filtered_parents)
+                children.append(child_node)
+
+            # Remove references to this node from its direct parents
+            for parent_edge in deleted_node.parents:
+                parent_node = parent_edge.destination_node
+                filtered_children = [
+                    edge
+                    for edge in parent_node.children
+                    if edge.destination_node.key != key
+                ]
+                parent_node.children.clear()
+                parent_node.children.extend(filtered_children)
+
+            del self.nodes[key]
+
+            # Enqueue children that have become orphans
+            for child in children:
+                if child.key != ROOT and child.key in self.nodes and not child.parents:
+                    queue.append(child.key)
 
     def get_dependency_edges(
         self, match_dep_types: list[RequirementType] | None = None

tests/test_dependency_graph.py

@@ -633,6 +633,70 @@ def test_remove_dependency_mid_graph_cascades() -> None:
     assert node_c.parents[0].destination_node.key == "a==1.0"
 
 
+def test_remove_dependency_shared_child_kept_by_other_subtree() -> None:
+    """Shared child and its descendants survive when kept alive by another subtree.
+
+    ROOT -> a -> b -> d
+           a -> c -> d
+    ROOT -> e -> c
+
+    Removing a should remove a, b; c and d survive because e still parents c.
+    """
+    graph = _build_graph(
+        ("ROOT", "a", "toplevel"),
+        ("ROOT", "e", "toplevel"),
+        ("a", "b", "install"),
+        ("a", "c", "install"),
+        ("b", "d", "install"),
+        ("c", "d", "install"),
+        ("e", "c", "install"),
+    )
+
+    graph.remove_dependency(canonicalize_name("a"), Version("1.0"))
+
+    assert "a==1.0" not in graph.nodes
+    assert "b==1.0" not in graph.nodes
+    assert "c==1.0" in graph.nodes
+    assert "d==1.0" in graph.nodes
+    assert "e==1.0" in graph.nodes
+
+    node_c = graph.nodes["c==1.0"]
+    assert len(node_c.parents) == 1
+    assert node_c.parents[0].destination_node.key == "e==1.0"
+    assert len(node_c.children) == 1
+    assert node_c.children[0].destination_node.key == "d==1.0"
+
+    node_d = graph.nodes["d==1.0"]
+    assert len(node_d.parents) == 1
+    assert node_d.parents[0].destination_node.key == "c==1.0"
+
+
+def test_remove_dependency_cascades_through_diamond() -> None:
+    """Single removal cascades through a diamond, removing all orphans.
+
+    ROOT -> a -> b -> d
+           a -> c -> d
+
+    Removing a orphans b and c; as both are removed, d loses all parents
+    and is also removed.
+    """
+    graph = _build_graph(
+        ("ROOT", "a", "toplevel"),
+        ("a", "b", "install"),
+        ("a", "c", "install"),
+        ("b", "d", "install"),
+        ("c", "d", "install"),
+    )
+
+    graph.remove_dependency(canonicalize_name("a"), Version("1.0"))
+
+    assert "a==1.0" not in graph.nodes
+    assert "b==1.0" not in graph.nodes
+    assert "c==1.0" not in graph.nodes
+    assert "d==1.0" not in graph.nodes
+    assert len(graph) == 0
+
+
 def test_remove_dependency_nonexistent() -> None:
     """Removing a node not in the graph is a no-op."""
     graph = _build_graph(("ROOT", "a", "toplevel"))