diff --git a/src/catkin_pkg/topological_order.py b/src/catkin_pkg/topological_order.py index 2bb396a6..6a39f29d 100644 --- a/src/catkin_pkg/topological_order.py +++ b/src/catkin_pkg/topological_order.py @@ -53,6 +53,8 @@ def __init__(self, package, path): self.depends_for_topological_order = None # a set containing this package name and recursive run_depends self._recursive_run_depends_for_topological_order = None + self.dependency_depth = 0 + self.dependency_fanout = 0 def __getattr__(self, name): if name.startswith('__'): @@ -120,6 +122,36 @@ def _add_recursive_run_depends(self, packages, depends_for_topological_order): depends_for_topological_order.update(self._recursive_run_depends_for_topological_order) + def set_dependency_depth(self, dependency_depth): + """ + Set dependency chain depth of this package. + + Dependency depth counts the worst-case length of the chain of packages which lead to + leaf nodes in the dependency tree. This is the primary sort criteria for package + build order. The idea is to build packages which start a long chain of dependencies + as soon as their dependencies are satisfied. This will (hopefully) on average satisfy + the dependencies of packages further down the chain sooner in the build process, leaving + fewer cases where a package late in the build process is the only job available + to run because it is holding up a number of additional dependencies. + + :param depth: Dependency depth to set for this package ``int`` + :returns: Nothing + """ + if self.dependency_depth < dependency_depth: # Want to keep worst-case dep chain length + self.dependency_depth = dependency_depth + + def inc_dependency_fanout(self): + """ + Increment the dependency_fanout field for this package. + + Fanout is a count of the number of packages which directly depend on this one. + Used as a secondary sort criteria / heuristic for determining which packages should + be built first. The concept is if two or more packages have the same depenency depth + then pick the one which should unblock the most dependend packages, keeping the + ready to build queue as full as possible. + """ + self.dependency_fanout += 1 + def topological_order(root_dir, whitelisted=None, blacklisted=None, underlay_workspaces=None): """ @@ -212,6 +244,72 @@ def topological_order_packages(packages, whitelisted=None, blacklisted=None, und return [(path, package) for path, package in tuples if path is None or package.name not in underlay_decorators_by_name] +def _set_dependency_depth(name, depth, packages): + """ + Update dependency chain depth for packge and all depencies. + + Traverse the dependency tree of a package, setting the dependency_depth of + the dependencies based on their depth in the dependency tree + + :param name: A string holding the name of the package ``str`` + :param depth: Integer depth from the root of the dependency tree + :param packages: A dict mapping package name to ``_PackageDecorator`` objects ``dict`` + :returns: nothing + """ + packages[name].set_dependency_depth(depth) + depth += 1 + for depend in packages[name].build_depends: + depend_str = str(depend) + if depend_str in packages: + packages[depend_str].set_dependency_depth(depth) + _set_dependency_depth(depend_str, depth, packages) + + +def _set_fanout(name, packages): + """ + Set fanout count for package - number of packages which depend on this one. + + Iterate the list of packages which are depenencies, incrementing + the fanout of each of those dependencies by 1. + :param name: A string holding the name of the package ``str`` + :param packages: A dict mapping package name to ``_PackageDecorator`` objects ``dict`` + :returns: nothing + """ + for depend in packages[name].build_depends: + depend_str = str(depend) + if depend_str in packages: + packages[depend_str].inc_dependency_fanout() + + +def _get_next_name(names, packages): + """ + Return the first name topolically sorting the named packages. + + Iterate through a list of package names, picking the package which should be added + next to the topolgical sort. + + The primary search criteria is dependency depth, the secondary one is the number of + packages which directly depend on a given package. See the comments in ``_PackageDecorator`` + for further discussion. + + :param names: A list of string holding the names of packages with 0 outstanding dependencies ``list`` + :param packages: A dict mapping package name to ``_PackageDecorator`` objects ``dict`` + :returns: A string holding the next package to add to the topological sort + """ + best_depth = -1 + best_fanout = 0 + best_name = '' + for this_name in names: + this_depth = packages[this_name].dependency_depth + this_fanout = packages[this_name].dependency_fanout + if (this_depth > best_depth) or ((this_depth == best_depth) and (this_fanout > best_fanout)): + best_depth = this_depth + best_fanout = this_fanout + best_name = this_name + + return best_name + + def _reduce_cycle_set(packages_orig): """ Remove iteratively some packages from a set that are definitely not part of any cycle. @@ -266,6 +364,10 @@ def _sort_decorated_packages(packages_orig): # queue for recursion dependency_names_to_follow.add(name) + for name in packages.keys(): + _set_dependency_depth(name, 0, packages) + _set_fanout(name, packages) + ordered_packages = [] while len(packages) > 0: # find all packages without build dependencies @@ -294,7 +396,7 @@ def _sort_decorated_packages(packages_orig): # add first candidates to ordered list # do not add all candidates since removing the depends from the first might affect the next candidates - name = names[0] + name = _get_next_name(names, packages) ordered_packages.append([packages[name].path, packages[name].package]) # remove package from further processing del packages[name] diff --git a/test/test_topological_order.py b/test/test_topological_order.py index 651cb6bb..9d5c00fa 100644 --- a/test/test_topological_order.py +++ b/test/test_topological_order.py @@ -7,7 +7,7 @@ try: from catkin_pkg.topological_order import topological_order_packages, _PackageDecorator, \ - _sort_decorated_packages + _sort_decorated_packages, _set_dependency_depth, _set_fanout except ImportError as e: raise ImportError('Please adjust your PYTHONPATH before running this test: %s' % str(e)) @@ -115,6 +115,78 @@ def create_mock(name, run_depends): self.assertEqual(set([mockproject1.name, mockproject4.name, mockproject5.name, mockproject6.name]), pd.depends_for_topological_order) + def test_set_dependency_depth(self): + def create_mock(name, build_depends): + m = Mock() + m.name = name + m.build_depends = build_depends + m.exports = [] + m.dependency_depth = 0 + return m + + mockproject1 = _PackageDecorator(create_mock('n1', []), 'p1') + mockproject2 = _PackageDecorator(create_mock('n2', [mockproject1.name]), 'p2') + mockproject3 = _PackageDecorator(create_mock('n3', [mockproject2.name]), 'p3') + mockproject4 = _PackageDecorator(create_mock('n4', [mockproject2.name]), 'p4') + mockproject5 = _PackageDecorator(create_mock('n5', [mockproject4.name]), 'p5') + mockproject6 = _PackageDecorator(create_mock('n6', [mockproject5.name]), 'p6') + + packages = {mockproject1.name: mockproject1, + mockproject2.name: mockproject2, + mockproject3.name: mockproject3, + mockproject4.name: mockproject4, + mockproject5.name: mockproject5, + mockproject6.name: mockproject6} + + for k in packages.keys(): + _set_dependency_depth(packages[k].name, 0, packages) + + # n1 -> n2 -> n3 + # \--> n4 -> n5 -> n6 + + # n1 - path n2 -> n4 -> n5 ->n6 == 4 (ignores shorter path n2->n3) + # n2 - path n4 -> n5 ->n6 == 3 + # n3 - leaf node == 0 + # n4 - path n5 ->n6 == 2 + # n5 - path n6 == 1 + # n6 - leaf node == 0 + self.assertEqual([4, 3, 0, 2, 1, 0], [packages[k].dependency_depth for k in sorted(packages.keys())]) + + def test_set_fanout(self): + def create_mock(name, build_depends): + m = Mock() + m.name = name + m.build_depends = build_depends + m.exports = [] + m.dependency_fanout = 0 + return m + + mockproject1 = _PackageDecorator(create_mock('n1', []), 'p1') + mockproject2 = _PackageDecorator(create_mock('n2', [mockproject1.name]), 'p2') + mockproject3 = _PackageDecorator(create_mock('n3', [mockproject2.name]), 'p3') + mockproject4 = _PackageDecorator(create_mock('n4', [mockproject2.name, mockproject3.name]), 'p4') + mockproject5 = _PackageDecorator(create_mock('n5', [mockproject4.name, mockproject3.name]), 'p5') + mockproject6 = _PackageDecorator(create_mock('n6', [mockproject5.name, mockproject2.name]), 'p6') + + packages = {mockproject1.name: mockproject1, + mockproject2.name: mockproject2, + mockproject3.name: mockproject3, + mockproject4.name: mockproject4, + mockproject5.name: mockproject5, + mockproject6.name: mockproject6} + + for k in packages.keys(): + _set_fanout(packages[k].name, packages) + + # count instances of mockprojectN.name in _PackageDecorator() constructors above + # n1 fans out to p2 + # n2 to p3, p4, p6 + # n3 to p4, p5 + # n4 to p5 + # n5 to p6 + # n6 to nothing + self.assertEqual([1, 3, 2, 1, 1, 0], [packages[k].dependency_fanout for k in sorted(packages.keys())]) + def test_sort_decorated_packages(self): projects = {} sprojects = _sort_decorated_packages(projects) @@ -123,8 +195,11 @@ def test_sort_decorated_packages(self): def create_mock(path): m = Mock() m.path = path + m.build_depends = [] m.depends_for_topological_order = set() m.message_generator = False + m.dependency_depth = 0 + m.dependency_fanout = 0 return m mock1 = create_mock('mock1') @@ -143,8 +218,11 @@ def test_sort_decorated_packages_favoring_message_generators(self): def create_mock(path): m = Mock() m.path = path + m.build_depends = [] m.depends_for_topological_order = set() m.message_generator = False + m.dependency_depth = 0 + m.dependency_fanout = 0 return m mock1 = create_mock('mock1') @@ -165,8 +243,11 @@ def test_sort_decorated_packages_cycles(self): def create_mock(path, depend): m = Mock() m.path = path + m.build_depends = [] m.depends_for_topological_order = set([depend]) m.message_generator = False + m.dependency_depth = 0 + m.dependency_fanout = 0 return m # creating a cycle for cycle detection