Use dependency chain depth as a secondary topological sort key

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]

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