From 74fbc9172911731f0dbf42a5d4489088291d913c Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Sat, 18 May 2019 11:00:51 -0700 Subject: [PATCH] Add a cache of targets to Context. --- src/python/pants/build_graph/build_graph.py | 30 ++++++++++++++++- src/python/pants/engine/legacy/graph.py | 15 --------- src/python/pants/goal/context.py | 36 +++++++++++++++------ 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/python/pants/build_graph/build_graph.py b/src/python/pants/build_graph/build_graph.py index 4a6af6e05d57..723a95d274e1 100644 --- a/src/python/pants/build_graph/build_graph.py +++ b/src/python/pants/build_graph/build_graph.py @@ -6,6 +6,7 @@ import itertools import logging +import weakref from abc import abstractmethod from builtins import filter, object from collections import defaultdict, deque @@ -98,6 +99,7 @@ def closure(*vargs, **kwargs): return Target.closure_for_targets(*vargs, **kwargs) def __init__(self): + self._invalidation_callbacks = [] self.reset() def __len__(self): @@ -126,6 +128,7 @@ def reset(self): :API: public """ + self._invalidated() self._target_by_address = OrderedDict() self._target_dependencies_by_address = defaultdict(OrderedSet) self._target_dependees_by_address = defaultdict(OrderedSet) @@ -133,6 +136,22 @@ def reset(self): self._derivatives_by_derived_from = defaultdict(list) # Address -> list of Address. self.synthetic_addresses = set() + def add_invalidation_callback(self, callback): + """Adds a no-arg function that will be called whenever the graph is changed. + + This interface exists to allow for invalidation of caches as the graph changes (due to injected + targets: generally synthetic). We should continue to move away from mutability in order to make + this kind of interface unnecessary, but synthetic targets are a fundamental part of the v1 model + that can likely only be removed via v2. + """ + self._invalidation_callbacks.append(weakref.ref(callback)) + + def _invalidated(self): + for callback_ref in self._invalidation_callbacks: + callback = callback_ref() + if callback is not None: + callback() + def contains_address(self, address): """ :API: public @@ -240,6 +259,7 @@ def inject_target(self, target, dependencies=None, derived_from=None, synthetic= :param bool synthetic: Whether to flag this target as synthetic, even if it isn't derived from another target. """ + self._invalidated() if self.contains_address(target.address): raise ValueError('Attempted to inject synthetic {target} derived from {derived_from}' ' into the BuildGraph with address {address}, but there is already a Target' @@ -290,6 +310,7 @@ def inject_dependency(self, dependent, dependency): is being added. :param Address dependency: The dependency to be injected. """ + self._invalidated() if dependent not in self._target_by_address: raise ValueError('Cannot inject dependency from {dependent} on {dependency} because the' ' dependent is not in the BuildGraph.' @@ -533,7 +554,6 @@ def transitive_subgraph_of_addresses_bfs(self, to_walk.append((level + 1, dep_address)) return ordered_closure - @abstractmethod def inject_synthetic_target(self, address, target_type, @@ -553,6 +573,14 @@ def inject_synthetic_target(self, from the `derived_from`. :param Target derived_from: The Target this Target will derive from. """ + target = target_type(name=address.target_name, + address=address, + build_graph=self, + **kwargs) + self.inject_target(target, + dependencies=dependencies, + derived_from=derived_from, + synthetic=True) def maybe_inject_address_closure(self, address): """If the given address is not already injected to the graph, calls inject_address_closure. diff --git a/src/python/pants/engine/legacy/graph.py b/src/python/pants/engine/legacy/graph.py index 9e361bfaa618..3cfe2c026515 100644 --- a/src/python/pants/engine/legacy/graph.py +++ b/src/python/pants/engine/legacy/graph.py @@ -180,21 +180,6 @@ def _instantiate_remote_sources(self, kwargs): kwargs['dest'] = _DestWrapper((self._target_types[kwargs['dest']],)) return RemoteSources(build_graph=self, **kwargs) - def inject_synthetic_target(self, - address, - target_type, - dependencies=None, - derived_from=None, - **kwargs): - target = target_type(name=address.target_name, - address=address, - build_graph=self, - **kwargs) - self.inject_target(target, - dependencies=dependencies, - derived_from=derived_from, - synthetic=True) - def inject_address_closure(self, address): self.inject_addresses_closure([address]) diff --git a/src/python/pants/goal/context.py b/src/python/pants/goal/context.py index 39498be0c00c..02061e193242 100644 --- a/src/python/pants/goal/context.py +++ b/src/python/pants/goal/context.py @@ -47,6 +47,8 @@ def __init__(self, options, run_tracker, target_roots, workspace=None, invalidation_report=None, scheduler=None): self._options = options self.build_graph = build_graph + self.build_graph.add_invalidation_callback(self._clear_target_cache) + self._targets_cache = dict() self._build_file_parser = build_file_parser self.build_configuration = build_configuration self.address_mapper = address_mapper @@ -269,6 +271,14 @@ def _replace_targets(self, target_roots): # only 1 remaining known use case in the Foursquare codebase that will be able to go away with # the post RoundEngine engine - kill the method at that time. self._target_roots = list(target_roots) + self._clear_target_cache() + + def _clear_target_cache(self): + """A callback for cases where the graph or target roots have been mutated. + + See BuildGraph.add_invalidation_callback. + """ + self._targets_cache.clear() def add_new_target(self, address, target_type, target_base=None, dependencies=None, derived_from=None, **kwargs): @@ -279,6 +289,7 @@ def add_new_target(self, address, target_type, target_base=None, dependencies=No :API: public """ + self._clear_target_cache() rel_target_base = target_base or address.spec_path abs_target_base = os.path.join(get_buildroot(), rel_target_base) if not os.path.exists(abs_target_base): @@ -317,21 +328,28 @@ def targets(self, predicate=None, **kwargs): `False` or preorder by default. :returns: A list of matching targets. """ - target_set = self._collect_targets(self.target_roots, **kwargs) + targets_cache_key = tuple(sorted(kwargs)) + targets = self._targets_cache.get(targets_cache_key) + if targets is None: + self._targets_cache[targets_cache_key] = targets = self._unfiltered_targets(**kwargs) + return list(filter(predicate, targets)) + + def _unfiltered_targets(self, **kwargs): + def _collect_targets(root_targets, **kwargs): + return Target.closure_for_targets( + target_roots=root_targets, + **kwargs + ) + + target_set = _collect_targets(self.target_roots, **kwargs) synthetics = OrderedSet() for synthetic_address in self.build_graph.synthetic_addresses: if self.build_graph.get_concrete_derived_from(synthetic_address) in target_set: synthetics.add(self.build_graph.get_target(synthetic_address)) - target_set.update(self._collect_targets(synthetics, **kwargs)) + target_set.update(_collect_targets(synthetics, **kwargs)) - return list(filter(predicate, target_set)) - - def _collect_targets(self, root_targets, **kwargs): - return Target.closure_for_targets( - target_roots=root_targets, - **kwargs - ) + return target_set def dependents(self, on_predicate=None, from_predicate=None): """Returns a map from targets that satisfy the from_predicate to targets they depend on that