From bd375fdd85b4e00b89fb087760369bb6f3df0a20 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 28 Jul 2017 21:33:42 +0000 Subject: [PATCH] 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 --- britney2/installability/solver.py | 112 +++++++++++++++++++++--------- tests/test_inst_tester.py | 31 +++++++++ 2 files changed, 110 insertions(+), 33 deletions(-) diff --git a/britney2/installability/solver.py b/britney2/installability/solver.py index 1ea9eed..2ec1587 100644 --- a/britney2/installability/solver.py +++ b/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): - """ - Tarjan's algorithm and topological sorting implementation in Python - - Find the strongly connected components in a graph using - Tarjan's algorithm. + def _compute_scc(self, order): + """Iterative algorithm for strongly-connected components - by Paul Harrison + Iterative variant of Tarjan's algorithm for finding strongly-connected + components. - Public domain, do with it as you will + :param order: Table of all nodes along which their ordering constraints + :return: List of components (each component is a list of items) """ - - result = [ ] - stack = [ ] - low = { } - - def visit(node): - if node in low: - return - - num = len(low) - low[node] = num - stack_pos = len(stack) - stack.append(node) - - for successor in order[node]['before']: - visit(successor) - low[node] = min(low[node], low[successor]) - - if num == low[node]: - component = tuple(stack[stack_pos:]) - del stack[stack_pos:] + result = [] + low = {} + node_stack = [] + + def _handle_succ(parent, parent_num, successors_remaining): + 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 + + 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 + + # This node is done; remove it from the work stack + work_stack.pop() + + # This node is out of successor. Push up the "low" value + # (Exception: root node has no parent) + node_num = low[node] + if work_stack: + parent = work_stack[-1][0] + parent_num = low[parent] + if node_num <= parent_num: + # This node is a part of a component with its parent. + # We update the parent's node number and push the + # responsibility of building the component unto the + # parent. + low[parent] = node_num + continue + if node_num != orig_node_num: + # The node is a part of an SCC with a ancestor (and parent) + continue + # We got a component + component = tuple(node_stack[stack_idx:]) + del node_stack[stack_idx:] result.append(component) for item in component: - low[item] = len(ptable) + low[item] = node_num - for node in order: - visit(node) + assert not node_stack return result diff --git a/tests/test_inst_tester.py b/tests/test_inst_tester.py index b2dc8cf..76980c6 100644 --- a/tests/test_inst_tester.py +++ b/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()