solver: Make _compute_scc iterative

Rewrite _compute_scc to be iterative to avoid call recursion limit for
graphs with long dependency chains.

Signed-off-by: Niels Thykier <niels@thykier.net>

britney2/installability/solver.py

@@ -182,7 +182,7 @@ class InstallabilitySolver(InstallabilityTester):
         #
         # Each one of those components will become an "easy" hint.
 
-        comps = self._compute_scc(order, ptable)
+        comps = self._compute_scc(order)
         merged = {}
         scc = {}
         # Now that we got the SSCs (in comps), we select on item from
@@ -269,44 +269,90 @@ class InstallabilitySolver(InstallabilityTester):
 
         return result
 
-    def _compute_scc(self, order, ptable):
+    def _compute_scc(self, order):
+        """Iterative algorithm for strongly-connected components
+
+        Iterative variant of Tarjan's algorithm for finding strongly-connected
+        components.
+
+        :param order: Table of all nodes along which their ordering constraints
+        :return: List of components (each component is a list of items)
         """
-        Tarjan's algorithm and topological sorting implementation in Python
+        result = []
+        low = {}
+        node_stack = []
 
-        Find the strongly connected components in a graph using
+        def _handle_succ(parent, parent_num, successors_remaining):
-        Tarjan's algorithm.
+            while successors_remaining:
+                succ = successors_remaining.pop()
+                succ_num = low.get(succ, None)
+                if succ_num is not None:
+                    if succ_num < parent_num:
+                        low[parent] = parent_num = succ_num
+                    continue
+                succ_num = len(low)
+                low[succ] = succ_num
+                # It cannot be a part of a SCC if it does not have depends
+                # or reverse depends.
+                if not order[succ]['before'] or not order[succ]['after']:
+                    # Short-cut obviously isolated component
+                    result.append((succ,))
+                    continue
+                work_stack.append((succ, len(node_stack), succ_num, order[succ]['before']))
+                node_stack.append(succ)
+                # "Recurse" into the child node first
+                return True
+            return False
 
-        by Paul Harrison
+        for n in order:
+            if n in low:
+                continue
+            root_num = len(low)
+            low[n] = root_num
+            # It cannot be a part of a SCC if it does not have depends
+            # or reverse depends.
+            if not order[n]['before'] or not order[n]['after']:
+                # Short-cut obviously isolated component
+                result.append((n,))
+                continue
+            # DFS work-stack needed to avoid call recursion.  It (more or less)
+            # replaces the variables on the call stack in Tarjan's algorithm
+            work_stack = [(n, len(node_stack), root_num, order[n]['before'])]
+            node_stack.append(n)
+            while work_stack:
+                node, stack_idx, orig_node_num, successors = work_stack[-1]
+                if successors and _handle_succ(node, low[node], successors):
+                    # _handle_succ has pushed a new node on to work_stack
+                    # and we need to "restart" the loop to handle that first
+                    continue
 
-        Public domain, do with it as you will
+                # This node is done; remove it from the work stack
-        """
+                work_stack.pop()
 
-        result = [ ]
+                # This node is out of successor.  Push up the "low" value
-        stack = [ ]
+                # (Exception: root node has no parent)
-        low = { }
+                node_num = low[node]
-
+                if work_stack:
-        def visit(node):
+                    parent = work_stack[-1][0]
-            if node in low:
+                    parent_num = low[parent]
-                return
+                    if node_num <= parent_num:
-
+                        # This node is a part of a component with its parent.
-            num = len(low)
+                        # We update the parent's node number and push the
-            low[node] = num
+                        # responsibility of building the component unto the
-            stack_pos = len(stack)
+                        # parent.
-            stack.append(node)
+                        low[parent] = node_num
-
+                        continue
-            for successor in order[node]['before']:
+                    if node_num != orig_node_num:
-                visit(successor)
+                        # The node is a part of an SCC with a ancestor (and parent)
-                low[node] = min(low[node], low[successor])
+                        continue
-
+                # We got a component
-            if num == low[node]:
+                component = tuple(node_stack[stack_idx:])
-                component = tuple(stack[stack_pos:])
+                del node_stack[stack_idx:]
-                del stack[stack_pos:]
                 result.append(component)
                 for item in component:
-                    low[item] = len(ptable)
+                    low[item] = node_num
 
-        for node in order:
+        assert not node_stack
-            visit(node)
 
         return result
 

tests/test_inst_tester.py

@@ -1,3 +1,4 @@
+import sys
 import unittest
 
 from . import new_pkg_universe_builder
@@ -412,6 +413,36 @@ class TestInstTester(unittest.TestCase):
         assert universe.stats.eqv_table_reduced_to_one == 0
         assert universe.stats.eqv_table_reduced_by_zero == 1
 
+    def test_solver_recursion_limit(self):
+        builder = new_pkg_universe_builder()
+        recursion_limit = 200
+        pkg_limit = recursion_limit + 20
+        orig_limit = sys.getrecursionlimit()
+        pkgs = [builder.new_package('pkg-%d' % i) for i in range(pkg_limit)]
+
+        for i, pkg in enumerate(pkgs):
+            # Intentionally -1 for the first package (wrap-around)
+            ni = i - 1
+            pkg.not_in_testing()
+            pkg.depends_on(pkgs[ni])
+
+        try:
+            sys.setrecursionlimit(recursion_limit)
+            universe = builder.build()
+            groups = []
+
+            for pkg in pkgs:
+                group = (pkg.pkg_id.package_name, {pkg.pkg_id}, set())
+                groups.append(group)
+
+            expected = {g[0] for g in groups}
+            actual = universe.solve_groups(groups)
+            assert actual
+            assert expected == set(actual[0])
+            assert len(actual) == 1
+        finally:
+            sys.setrecursionlimit(orig_limit)
+
     def test_solver_simple_scc(self):
         builder = new_pkg_universe_builder()