diff options
3 files changed, 36 insertions, 23 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 3786ddcb4e..303659dcf5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -87,8 +87,6 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver stateMap = Maps.newConcurrentMap(); } - private static final Function<String, SkyKey> VAR_TO_SKYKEY = ClientEnvironmentFunction::key; - @Override public SkyValue compute(SkyKey skyKey, Environment env) throws ActionExecutionFunctionException, InterruptedException { @@ -111,7 +109,9 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver // Look up the parts of the environment that influence the action. Map<SkyKey, SkyValue> clientEnvLookup = - env.getValues(Iterables.transform(action.getClientEnvironmentVariables(), VAR_TO_SKYKEY)); + env.getValues( + Iterables.transform( + action.getClientEnvironmentVariables(), ClientEnvironmentFunction::key)); if (env.valuesMissing()) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java index bb005339fd..4357e2344c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ClientEnvironmentValue.java @@ -15,9 +15,13 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.skyframe.SkyValue; +import java.util.Objects; import javax.annotation.Nullable; -/** An aspect in the context of the Skyframe graph. */ +/** + * The value of an environmental variable from the client environment. These are invalidated and + * injected by {@link SequencedSkyframeExecutor}. + */ public final class ClientEnvironmentValue implements SkyValue { private final String value; @@ -30,4 +34,15 @@ public final class ClientEnvironmentValue implements SkyValue { public String getValue() { return value; } + + @Override + public boolean equals(Object o) { + return (o instanceof ClientEnvironmentValue) + && Objects.equals(((ClientEnvironmentValue) o).value, value); + } + + @Override + public int hashCode() { + return (value != null) ? value.hashCode() : 0; + } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 6844818b7c..88cced7588 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; -import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.Collections2; @@ -131,7 +130,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { private final RecordingDifferencer recordingDiffer = new SequencedRecordingDifferencer(); private final DiffAwarenessManager diffAwarenessManager; private final Iterable<SkyValueDirtinessChecker> customDirtinessCheckers; - private Set<String> previousClientEnvironment = null; + private Set<String> previousClientEnvironment = ImmutableSet.of(); private SequencedSkyframeExecutor( EvaluatorSupplier evaluatorSupplier, @@ -339,25 +338,24 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { handleClientEnvironmentChanges(); } - /** Invalidates entries in the client environment that have changed since last sync. */ + /** Invalidates entries in the client environment. */ private void handleClientEnvironmentChanges() { - Map<SkyKey, SkyValue> values = memoizingEvaluator.getValues(); - ImmutableMap.Builder<SkyKey, SkyValue> newValuesBuilder = ImmutableMap.builder(); - HashSet<String> envToCheck = new HashSet<>(); - if (previousClientEnvironment != null) { - envToCheck.addAll(previousClientEnvironment); - } - envToCheck.addAll(clientEnv.get().keySet()); + // Remove deleted client environmental variables. + Iterable<SkyKey> deletedKeys = + Sets.difference(previousClientEnvironment, clientEnv.get().keySet()) + .stream() + .map(ClientEnvironmentFunction::key) + .collect(ImmutableList.toImmutableList()); + recordingDiffer.invalidate(deletedKeys); previousClientEnvironment = clientEnv.get().keySet(); - for (String env : envToCheck) { - SkyKey key = ClientEnvironmentFunction.key(env); - if (values.containsKey(key)) { - String value = ((ClientEnvironmentValue) values.get(key)).getValue(); - String newValue = clientEnv.get().get(env); - if (!Objects.equal(newValue, value)) { - newValuesBuilder.put(key, new ClientEnvironmentValue(newValue)); - } - } + // Inject current client environmental values. We can inject unconditionally without fearing + // over-invalidation; skyframe will not invalidate an injected key if the key's new value is the + // same as the old value. + ImmutableMap.Builder<SkyKey, SkyValue> newValuesBuilder = ImmutableMap.builder(); + for (Map.Entry<String, String> entry : clientEnv.get().entrySet()) { + newValuesBuilder.put( + ClientEnvironmentFunction.key(entry.getKey()), + new ClientEnvironmentValue(entry.getValue())); } recordingDiffer.inject(newValuesBuilder.build()); } |