diff options
author | 2016-08-02 18:13:28 +0000 | |
---|---|---|
committer | 2016-08-03 07:57:26 +0000 | |
commit | c55fe15fc36ad01b93f4efe85ff85911d041d5d7 (patch) | |
tree | ae8ec9fc39f54ca08703ee82fbc85e3a5acd519a /src/main/java | |
parent | 70fbf690e571037370044f7d1e316b0bf9172e1c (diff) |
Delete NodeEntryField since it's now superfluous in the presence of the new QueryableGraph.Reason which conveys more information. Add a few more Reason enum values to make this refactor benign.
--
MOS_MIGRATED_REVID=129118462
Diffstat (limited to 'src/main/java')
7 files changed, 55 insertions, 101 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java index b6c582559e..c56bf55132 100644 --- a/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java +++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingWalkableGraph.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.skyframe.QueryableGraph.Reason; -import java.util.EnumSet; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -36,11 +35,10 @@ public class DelegatingWalkableGraph implements WalkableGraph { this.graph = graph; } - private NodeEntry getEntry(SkyKey key) { + private NodeEntry getEntryForValue(SkyKey key) { NodeEntry entry = Preconditions.checkNotNull( - graph.getBatchWithFieldHints( - null, Reason.OTHER, ImmutableList.of(key), NodeEntryField.VALUE_ONLY).get(key), + graph.getBatch(null, Reason.WALKABLE_GRAPH_VALUE, ImmutableList.of(key)).get(key), key); Preconditions.checkState(entry.isDone(), "%s %s", key, entry); return entry; @@ -49,15 +47,14 @@ public class DelegatingWalkableGraph implements WalkableGraph { @Override public boolean exists(SkyKey key) { NodeEntry entry = - graph.getBatchWithFieldHints( - null, Reason.OTHER, ImmutableList.of(key), NodeEntryField.NO_FIELDS).get(key); + graph.getBatch(null, Reason.EXISTENCE_CHECKING, ImmutableList.of(key)).get(key); return entry != null && entry.isDone(); } @Nullable @Override public SkyValue getValue(SkyKey key) { - return getEntry(key).getValue(); + return getEntryForValue(key).getValue(); } private static final Function<NodeEntry, SkyValue> GET_SKY_VALUE_FUNCTION = @@ -73,7 +70,7 @@ public class DelegatingWalkableGraph implements WalkableGraph { public Map<SkyKey, SkyValue> getSuccessfulValues(Iterable<SkyKey> keys) { return Maps.filterValues( Maps.transformValues( - graph.getBatchWithFieldHints(null, Reason.OTHER, keys, NodeEntryField.VALUE_ONLY), + graph.getBatch(null, Reason.WALKABLE_GRAPH_VALUE, keys), GET_SKY_VALUE_FUNCTION), Predicates.notNull()); } @@ -81,8 +78,7 @@ public class DelegatingWalkableGraph implements WalkableGraph { @Override public Map<SkyKey, Exception> getMissingAndExceptions(Iterable<SkyKey> keys) { Map<SkyKey, Exception> result = new HashMap<>(); - Map<SkyKey, NodeEntry> graphResult = - graph.getBatchWithFieldHints(null, Reason.OTHER, keys, NodeEntryField.VALUE_ONLY); + Map<SkyKey, NodeEntry> graphResult = graph.getBatch(null, Reason.WALKABLE_GRAPH_VALUE, keys); for (SkyKey key : keys) { NodeEntry nodeEntry = graphResult.get(key); if (nodeEntry == null || !nodeEntry.isDone()) { @@ -100,14 +96,13 @@ public class DelegatingWalkableGraph implements WalkableGraph { @Nullable @Override public Exception getException(SkyKey key) { - ErrorInfo errorInfo = getEntry(key).getErrorInfo(); + ErrorInfo errorInfo = getEntryForValue(key).getErrorInfo(); return errorInfo == null ? null : errorInfo.getException(); } @Override public Map<SkyKey, Iterable<SkyKey>> getDirectDeps(Iterable<SkyKey> keys) { - Map<SkyKey, NodeEntry> entries = graph.getBatchWithFieldHints( - null, Reason.OTHER, keys, EnumSet.of(NodeEntryField.DIRECT_DEPS)); + Map<SkyKey, NodeEntry> entries = graph.getBatch(null, Reason.WALKABLE_GRAPH_DEPS, keys); Map<SkyKey, Iterable<SkyKey>> result = new HashMap<>(entries.size()); for (Entry<SkyKey, NodeEntry> entry : entries.entrySet()) { Preconditions.checkState(entry.getValue().isDone(), entry); @@ -118,8 +113,7 @@ public class DelegatingWalkableGraph implements WalkableGraph { @Override public Map<SkyKey, Iterable<SkyKey>> getReverseDeps(Iterable<SkyKey> keys) { - Map<SkyKey, NodeEntry> entries = graph.getBatchWithFieldHints( - null, Reason.OTHER, keys, EnumSet.of(NodeEntryField.REVERSE_DEPS)); + Map<SkyKey, NodeEntry> entries = graph.getBatch(null, Reason.WALKABLE_GRAPH_RDEPS, keys); Map<SkyKey, Iterable<SkyKey>> result = new HashMap<>(entries.size()); for (Entry<SkyKey, NodeEntry> entry : entries.entrySet()) { Preconditions.checkState(entry.getValue().isDone(), entry); diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java b/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java index 0db7daddf9..6e93ce4ce3 100644 --- a/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java +++ b/src/main/java/com/google/devtools/build/skyframe/EvaluableGraph.java @@ -24,7 +24,7 @@ import javax.annotation.Nullable; @ThreadSafe interface EvaluableGraph extends QueryableGraph, DeletableGraph { /** - * Like {@link QueryableGraph#getBatchWithFieldHints}, except it creates a new node for each key + * Like {@link QueryableGraph#getBatch}, except it creates a new node for each key * not already present in the graph. Thus, the returned map will have an entry for each key in * {@code keys}. * diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java index e068c3f4ad..ff5ce57b69 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java @@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapMaker; import com.google.common.collect.Maps; import java.util.Collections; -import java.util.EnumSet; import java.util.Map; import java.util.concurrent.ConcurrentMap; import javax.annotation.Nullable; @@ -68,8 +67,7 @@ public class InMemoryGraphImpl implements InMemoryGraph { } @Override - public Map<SkyKey, NodeEntry> getBatchWithFieldHints( - SkyKey requestor, Reason reason, Iterable<SkyKey> keys, EnumSet<NodeEntryField> fields) { + public Map<SkyKey, NodeEntry> getBatch(SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { return getBatchForInvalidation(keys); } diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntryField.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntryField.java deleted file mode 100644 index 97c1dfd87c..0000000000 --- a/src/main/java/com/google/devtools/build/skyframe/NodeEntryField.java +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.skyframe; - -import java.util.EnumSet; - -/** - * Fields of a {@link NodeEntry} that clients of a {@link QueryableGraph} may need. Clients may - * specify these fields in {@link QueryableGraph#getBatchWithFieldHints} to help particular {@link - * QueryableGraph} implementations decide how lazily to construct the returned node entries. - */ -public enum NodeEntryField { - /** The value ({@link NodeEntry#getValueMaybeWithMetadata}) will be needed. */ - VALUE, - /** The direct deps ({@link NodeEntry#getDirectDeps}) will be needed. */ - DIRECT_DEPS, - /** The reverse deps ({@link NodeEntry#getReverseDeps}) will be needed. */ - REVERSE_DEPS, - /** - * The reverse deps as a whole will not be needed, but we may need to check the presence of a - * reverse dep or add/delete one. - */ - INDIVIDUAL_REVERSE_DEPS; - - public static final EnumSet<NodeEntryField> NO_FIELDS = EnumSet.noneOf(NodeEntryField.class); - public static final EnumSet<NodeEntryField> VALUE_ONLY = EnumSet.of(VALUE); - public static final EnumSet<NodeEntryField> NO_VALUE = EnumSet.of(DIRECT_DEPS, REVERSE_DEPS); - public static final EnumSet<NodeEntryField> ALL_FIELDS = - EnumSet.of(VALUE, DIRECT_DEPS, REVERSE_DEPS); -} diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index 61c93cb757..ea83d875cc 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -57,7 +57,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Deque; -import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -227,7 +226,7 @@ public final class ParallelEvaluator implements Evaluator { SkyKey parent, Reason reason, Iterable<SkyKey> keys) { - return graph.getBatchWithFieldHints(parent, reason, keys, NodeEntryField.VALUE_ONLY); + return graph.getBatch(parent, reason, keys); } /** Receives the events from the NestedSet and delegates to the reporter. */ @@ -667,11 +666,10 @@ public final class ParallelEvaluator implements Evaluator { // Remove the rdep on this entry for each of its old deps that is no longer a direct dep. Set<SkyKey> depsToRemove = Sets.difference(oldDeps, primaryEntry.getTemporaryDirectDeps().toSet()); - Collection<NodeEntry> oldDepEntries = graph.getBatchWithFieldHints( + Collection<NodeEntry> oldDepEntries = graph.getBatch( skyKey, Reason.RDEP_REMOVAL, - depsToRemove, - EnumSet.of(NodeEntryField.INDIVIDUAL_REVERSE_DEPS)).values(); + depsToRemove).values(); for (NodeEntry oldDepEntry : oldDepEntries) { oldDepEntry.removeReverseDep(skyKey); } @@ -935,11 +933,7 @@ public final class ParallelEvaluator implements Evaluator { // We check the deps for errors so that we don't continue building this node if it has // a child error. Map<SkyKey, NodeEntry> entriesToCheck = - graph.getBatchWithFieldHints( - skyKey, - Reason.EVALUATION, - directDepsToCheck, - EnumSet.of(NodeEntryField.VALUE, NodeEntryField.INDIVIDUAL_REVERSE_DEPS)); + graph.getBatch(skyKey, Reason.OTHER, directDepsToCheck); for (Map.Entry<SkyKey, NodeEntry> entry : entriesToCheck.entrySet()) { if (entry.getValue().isDone() && entry.getValue().getErrorInfo() != null) { // If any child has an error, we arbitrarily add a dep on the first one (needed @@ -1111,11 +1105,7 @@ public final class ParallelEvaluator implements Evaluator { registerNewlyDiscoveredDepsForDoneEntry( skyKey, state, - graph.getBatchWithFieldHints( - skyKey, - Reason.RDEP_ADDITION, - env.newlyRequestedDeps, - EnumSet.of(NodeEntryField.INDIVIDUAL_REVERSE_DEPS)), + graph.getBatch(skyKey, Reason.RDEP_ADDITION, env.newlyRequestedDeps), oldDeps, env); env.commit(state, /*enqueueParents=*/true); @@ -1241,7 +1231,7 @@ public final class ParallelEvaluator implements Evaluator { // importantly, these hints are not respected for not-done nodes. If they are, we may need to // alter this hint. Map<SkyKey, NodeEntry> batch = - graph.getBatchWithFieldHints(skyKey, Reason.SIGNAL_DEP, keys, NodeEntryField.NO_FIELDS); + graph.getBatch(skyKey, Reason.SIGNAL_DEP, keys); if (visitor != null) { for (SkyKey key : keys) { NodeEntry entry = Preconditions.checkNotNull(batch.get(key), key); @@ -1339,7 +1329,7 @@ public final class ParallelEvaluator implements Evaluator { // directly without launching the heavy machinery, spawning threads, etc. // Inform progressReceiver that these nodes are done to be consistent with the main code path. boolean allAreDone = true; - Map<SkyKey, NodeEntry> batch = getBatchValues(null, Reason.EVALUATION, skyKeySet); + Map<SkyKey, NodeEntry> batch = getBatchValues(null, Reason.PRE_OR_POST_EVALUATION, skyKeySet); for (SkyKey key : skyKeySet) { if (!isDoneForBuild(batch.get(key))) { allAreDone = false; @@ -1358,7 +1348,7 @@ public final class ParallelEvaluator implements Evaluator { if (!keepGoing) { Set<SkyKey> cachedErrorKeys = new HashSet<>(); for (SkyKey skyKey : skyKeySet) { - NodeEntry entry = graph.get(null, Reason.EVALUATION, skyKey); + NodeEntry entry = graph.get(null, Reason.PRE_OR_POST_EVALUATION, skyKey); if (entry == null) { continue; } @@ -1402,9 +1392,10 @@ public final class ParallelEvaluator implements Evaluator { // We unconditionally add the ErrorTransienceValue here, to ensure that it will be created, and // in the graph, by the time that it is needed. Creating it on demand in a parallel context sets // up a race condition, because there is no way to atomically create a node and set its value. - NodeEntry errorTransienceEntry = Iterables.getOnlyElement( - graph.createIfAbsentBatch( - null, Reason.EVALUATION, ImmutableList.of(ErrorTransienceValue.KEY)).values()); + NodeEntry errorTransienceEntry = Iterables.getOnlyElement(graph.createIfAbsentBatch( + null, + Reason.PRE_OR_POST_EVALUATION, + ImmutableList.of(ErrorTransienceValue.KEY)).values()); if (!errorTransienceEntry.isDone()) { injectValues( ImmutableMap.of(ErrorTransienceValue.KEY, (SkyValue) ErrorTransienceValue.INSTANCE), @@ -1413,7 +1404,7 @@ public final class ParallelEvaluator implements Evaluator { dirtyKeyTracker); } for (Map.Entry<SkyKey, NodeEntry> e - : graph.createIfAbsentBatch(null, Reason.EVALUATION, skyKeys).entrySet()) { + : graph.createIfAbsentBatch(null, Reason.PRE_OR_POST_EVALUATION, skyKeys).entrySet()) { SkyKey skyKey = e.getKey(); NodeEntry entry = e.getValue(); // This must be equivalent to the code in enqueueChild above, in order to be thread-safe. @@ -1686,7 +1677,7 @@ public final class ParallelEvaluator implements Evaluator { for (SkyKey skyKey : skyKeys) { SkyValue unwrappedValue = maybeGetValueFromError( skyKey, - graph.get(null, Reason.EVALUATION, skyKey), + graph.get(null, Reason.PRE_OR_POST_EVALUATION, skyKey), bubbleErrorInfo); ValueWithMetadata valueWithMetadata = unwrappedValue == NULL_MARKER ? null : ValueWithMetadata.wrapWithMetadata(unwrappedValue); @@ -1937,8 +1928,8 @@ public final class ParallelEvaluator implements Evaluator { // out. // TODO(janakr): If graph implementations start using these hints for not-done nodes, we may // have to change this. - Map<SkyKey, NodeEntry> childrenNodes = graph.getBatchWithFieldHints( - key, Reason.CYCLE_CHECKING, children, NodeEntryField.NO_FIELDS); + Map<SkyKey, NodeEntry> childrenNodes = + graph.getBatch(key, Reason.EXISTENCE_CHECKING, children); Preconditions.checkState(childrenNodes.size() == Iterables.size(children), childrenNodes); children = Maps.filterValues(childrenNodes, new Predicate<NodeEntry>() { @Override diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java index 75ce05c970..f76c0aeeb7 100644 --- a/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java +++ b/src/main/java/com/google/devtools/build/skyframe/QueryableGraph.java @@ -14,8 +14,6 @@ package com.google.devtools.build.skyframe; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; - -import java.util.EnumSet; import java.util.Map; import javax.annotation.Nullable; @@ -36,20 +34,13 @@ public interface QueryableGraph { /** * Fetches all the given nodes. Returns a map {@code m} such that, for all {@code k} in {@code * keys}, {@code m.get(k).equals(e)} iff {@code get(k) == e} and {@code e != null}, and {@code - * !m.containsKey(k)} iff {@code get(k) == null}. The {@code fields} parameter is a hint to the - * QueryableGraph implementation that allows it to possibly construct certain fields of the - * returned node entries more lazily. Hints may only be applied to nodes in a certain state, like - * done nodes. + * !m.containsKey(k)} iff {@code get(k) == null}. * * @param requestor if non-{@code null}, the node on behalf of which the given {@code keys} are * being requested. * @param reason the reason the nodes are being requested. */ - Map<SkyKey, NodeEntry> getBatchWithFieldHints( - @Nullable SkyKey requestor, - Reason reason, - Iterable<SkyKey> keys, - EnumSet<NodeEntryField> fields); + Map<SkyKey, NodeEntry> getBatch(@Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys); /** * The reason that a node is being looked up in the Skyframe graph. @@ -58,14 +49,20 @@ public interface QueryableGraph { */ enum Reason { /** + * The node is being fetched in order to see if it needs to be evaluated or because it was just + * evaluated, but *not* because it was just requested during evaluation of a SkyFunction + * (see {@link #DEP_REQUESTED}). + */ + PRE_OR_POST_EVALUATION, + + /** * The node is being looked up as part of the prefetch step before evaluation of a SkyFunction. */ PREFETCH, /** - * The node is being fetched because it is about to be evaluated or it has already been - * evaluated, but *not* because it was just requested during evaluation of a SkyFunction (see - * DEP_REQUESTED). + * The node is being fetched because it is about to be evaluated, but *not* because it was just + * requested during evaluation of a SkyFunction (see {@link #DEP_REQUESTED}). */ EVALUATION, @@ -98,6 +95,22 @@ public interface QueryableGraph { */ ERROR_BUBBLING, + /** The node is being looked up merely for an existence check. */ + EXISTENCE_CHECKING, + + /** + * The node is being looked up to service {@link WalkableGraph#getValue}, + * {@link WalkableGraph#getException}, {@link WalkableGraph#getMissingAndExceptions}, or + * {@link WalkableGraph#getSuccessfulValues}. + */ + WALKABLE_GRAPH_VALUE, + + /** The node is being looked up to service {@link WalkableGraph#getDirectDeps}. */ + WALKABLE_GRAPH_DEPS, + + /** The node is being looked up to service {@link WalkableGraph#getReverseDeps}. */ + WALKABLE_GRAPH_RDEPS, + /** Some other reason than one of the above. */ OTHER, } diff --git a/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java index 6aaadcd80a..79ed64056a 100644 --- a/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/QueryableGraphBackedSkyFunctionEnvironment.java @@ -70,8 +70,7 @@ public class QueryableGraphBackedSkyFunctionEnvironment extends AbstractSkyFunct @Override protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(Set<SkyKey> depKeys) { - Map<SkyKey, NodeEntry> resultMap = queryableGraph.getBatchWithFieldHints( - null, Reason.DEP_REQUESTED, depKeys, NodeEntryField.VALUE_ONLY); + Map<SkyKey, NodeEntry> resultMap = queryableGraph.getBatch(null, Reason.DEP_REQUESTED, depKeys); Map<SkyKey, NodeEntry> resultWithMissingKeys = new HashMap<>(resultMap); for (SkyKey missingDep : Sets.difference(depKeys, resultMap.keySet())) { resultWithMissingKeys.put(missingDep, null); |