[buildstream] 04/12: _pipeline.py: Drop the optimization for cached elements in the planner

[no...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

not-in-ldap pushed a commit to branch juerg/cache-query-job-benchmark
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit afd578ba876b608cb88b875b688c611bdde5eec4
Author: Jürg Billeter <j...@bitron.ch>
AuthorDate: Wed Sep 16 15:55:26 2020 +0200

    _pipeline.py: Drop the optimization for cached elements in the planner
    
    The overhead of planning already cached elements and unneeded build-only
    dependencies should be fairly small as unneeded jobs can still be
    skipped. This optimization was also already disabled for non-strict
    build plans with a remote artifact cache.
    
    This change is necessary in preparation for parallelizing cache queries.
---
 src/buildstream/_pipeline.py | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/buildstream/_pipeline.py b/src/buildstream/_pipeline.py
index d53fc9d..7aec985 100644
--- a/src/buildstream/_pipeline.py
+++ b/src/buildstream/_pipeline.py
@@ -141,9 +141,8 @@ class Pipeline:
     # plan()
     #
     # Generator function to iterate over only the elements
-    # which are required to build the pipeline target, omitting
-    # cached elements. The elements are yielded in a depth sorted
-    # ordering for optimal build plans
+    # which are required to build the pipeline target The elements are
+    # yielded in a depth sorted ordering for optimal build plans
     #
     # Args:
     #    elements (list of Element): List of target elements to plan
@@ -152,11 +151,7 @@ class Pipeline:
     #    (list of Element): A depth sorted list of the build plan
     #
     def plan(self, elements):
-        # Keep locally cached elements in the plan if remote artifact cache is used
-        # to allow pulling artifact with strict cache key, if available.
-        plan_cached = not self._context.get_strict() and self._artifacts.has_fetch_remotes()
-
-        return _Planner().plan(elements, plan_cached)
+        return _Planner().plan(elements)
 
     # get_selection()
     #
@@ -421,9 +416,8 @@ class Pipeline:
 # _Planner()
 #
 # An internal object used for constructing build plan
-# from a given resolved toplevel element, while considering what
-# parts need to be built depending on build only dependencies
-# being cached, and depth sorting for more efficient processing.
+# from a given resolved toplevel element, using depth
+# sorting for more efficient processing.
 #
 class _Planner:
     def __init__(self):
@@ -447,15 +441,13 @@ class _Planner:
         for dep in element._dependencies(_Scope.RUN, recurse=False):
             self.plan_element(dep, depth)
 
-        # Dont try to plan builds of elements that are cached already
-        if not element._cached_success():
-            for dep in element._dependencies(_Scope.BUILD, recurse=False):
-                self.plan_element(dep, depth + 1)
+        for dep in element._dependencies(_Scope.BUILD, recurse=False):
+            self.plan_element(dep, depth + 1)
 
         self.depth_map[element] = depth
         self.visiting_elements.remove(element)
 
-    def plan(self, roots, plan_cached):
+    def plan(self, roots):
         for root in roots:
             self.plan_element(root, 0)
 
@@ -465,4 +457,4 @@ class _Planner:
         for index, item in enumerate(depth_sorted):
             item[0]._set_depth(index)
 
-        return [item[0] for item in depth_sorted if plan_cached or not item[0]._cached_success()]
+        return [item[0] for item in depth_sorted]