From 73876205d11794a5b748dc3e800261c0f459ef76 Mon Sep 17 00:00:00 2001 From: Rumou Duan Date: Mon, 6 Jun 2016 18:52:08 +0000 Subject: Skyframe changes to support SpawnActionTemplate. 1. Adds ActionTemplateExpansion{Function, Value} for ActionTemplate expansion. 2. Changes ArtifactFunction to support evaluating TreeFileArtifacts and TreeArtifacts generated by ActionTemplate. -- MOS_MIGRATED_REVID=124160939 --- .../build/lib/skyframe/ActionExecutionValue.java | 15 +++ .../build/lib/skyframe/ActionMetadataHandler.java | 20 +-- .../skyframe/ActionTemplateExpansionFunction.java | 88 +++++++++++++ .../lib/skyframe/ActionTemplateExpansionValue.java | 100 +++++++++++++++ .../build/lib/skyframe/ArtifactFunction.java | 141 +++++++++++++++++---- .../devtools/build/lib/skyframe/SkyFunctions.java | 2 + .../build/lib/skyframe/SkyframeExecutor.java | 1 + 7 files changed, 325 insertions(+), 42 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java (limited to 'src/main/java/com/google/devtools') diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java index e56ef46f6d..4c3c6a2c93 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java @@ -14,9 +14,11 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.actions.Artifact; @@ -36,6 +38,15 @@ import javax.annotation.Nullable; @Immutable @ThreadSafe public class ActionExecutionValue implements SkyValue { + + private static final Function TO_KEY = + new Function() { + @Override + public SkyKey apply(Action action) { + return key(action); + } + }; + /* Concerning the data in this class: @@ -131,6 +142,10 @@ public class ActionExecutionValue implements SkyValue { return SkyKey.create(SkyFunctions.ACTION_EXECUTION, action); } + static Iterable keys(Iterable actions) { + return Iterables.transform(actions, TO_KEY); + } + /** * Returns whether the key corresponds to a ActionExecutionValue worth reporting status about. * diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index 28f7d0c731..a72ef48557 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java @@ -102,16 +102,6 @@ public class ActionMetadataHandler implements MetadataHandler { private final ConcurrentMap additionalOutputData = new ConcurrentHashMap<>(); - /** - * Contains per-fragment FileArtifactValues when those values must be stored separately. - * Bona-fide Artifacts are stored in {@link #additionalOutputData} instead. - * See {@link #getAdditionalOutputData()} for details. - * Unlike additionalOutputData, this map is discarded (the relevant FileArtifactValues - * are stored in outputTreeArtifactData's values instead). - */ - private final ConcurrentMap cachedTreeFileArtifactData = - new ConcurrentHashMap<>(); - /** * Data for TreeArtifactValues, constructed from outputArtifactData and * additionalOutputFileData. @@ -287,8 +277,7 @@ public class ActionMetadataHandler implements MetadataHandler { // We are dealing with artifacts inside a tree artifact. FileArtifactValue value = FileArtifactValue.createWithDigest(artifact.getPath(), injectedDigest, data.getSize()); - FileArtifactValue oldValue = cachedTreeFileArtifactData.putIfAbsent( - (TreeFileArtifact) artifact, value); + FileArtifactValue oldValue = additionalOutputData.putIfAbsent(artifact, value); checkInconsistentData(artifact, oldValue, value); return new Metadata(value.getDigest()); } @@ -375,7 +364,7 @@ public class ActionMetadataHandler implements MetadataHandler { Maps.newHashMapWithExpectedSize(contents.size()); for (TreeFileArtifact treeFileArtifact : contents) { - FileArtifactValue cachedValue = cachedTreeFileArtifactData.get(treeFileArtifact); + FileArtifactValue cachedValue = additionalOutputData.get(treeFileArtifact); if (cachedValue == null) { FileValue fileValue = outputArtifactData.get(treeFileArtifact); // This is similar to what's present in getRealMetadataForArtifact, except @@ -385,11 +374,11 @@ public class ActionMetadataHandler implements MetadataHandler { if (fileValue == null) { fileValue = constructFileValue(treeFileArtifact, /*statNoFollow=*/ null); // A minor hack: maybeStoreAdditionalData will force the data to be stored - // in cachedTreeFileArtifactData. + // in additionalOutputData. maybeStoreAdditionalData(treeFileArtifact, fileValue, null); } cachedValue = Preconditions.checkNotNull( - cachedTreeFileArtifactData.get(treeFileArtifact), treeFileArtifact); + additionalOutputData.get(treeFileArtifact), treeFileArtifact); } values.put(treeFileArtifact, cachedValue); @@ -500,7 +489,6 @@ public class ActionMetadataHandler implements MetadataHandler { outputDirectoryListings.clear(); outputTreeArtifactData.clear(); additionalOutputData.clear(); - cachedTreeFileArtifactData.clear(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java new file mode 100644 index 0000000000..26d254b9eb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java @@ -0,0 +1,88 @@ +// 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.lib.skyframe; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; +import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; +import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +import javax.annotation.Nullable; + +/** + * The SkyFunction for {@link ActionTemplateExpansionValue}. + * + *

Given an action template, this function resolves its input TreeArtifact, then expands the + * action template into a list of actions using the expanded {@link TreeFileArtifact}s under the + * input TreeArtifact. + */ +public class ActionTemplateExpansionFunction implements SkyFunction { + + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws ActionTemplateExpansionFunctionException { + ActionTemplateExpansionKey key = (ActionTemplateExpansionKey) skyKey.argument(); + SpawnActionTemplate actionTemplate = key.getActionTemplate(); + + // Requests the TreeArtifactValue object for the input TreeArtifact. + SkyKey artifactValueKey = ArtifactValue.key(actionTemplate.getInputTreeArtifact(), true); + TreeArtifactValue treeArtifactValue = (TreeArtifactValue) env.getValue(artifactValueKey); + + // Input TreeArtifact is not ready yet. + if (env.valuesMissing()) { + return null; + } + Iterable inputTreeFileArtifacts = treeArtifactValue.getChildren(); + Iterable expandedActions; + try { + // Expand the action template using the list of expanded input TreeFileArtifacts. + expandedActions = ImmutableList.copyOf( + actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key)); + } catch (ActionConflictException e) { + e.reportTo(env.getListener()); + throw new ActionTemplateExpansionFunctionException(e); + } catch (ArtifactPrefixConflictException e) { + env.getListener().handle(Event.error(e.getMessage())); + throw new ActionTemplateExpansionFunctionException(e); + } + + return new ActionTemplateExpansionValue(expandedActions); + } + + /** Exception thrown by {@link ActionTemplateExpansionFunction}. */ + public static final class ActionTemplateExpansionFunctionException extends SkyFunctionException { + ActionTemplateExpansionFunctionException(ActionConflictException e) { + super(e, Transience.PERSISTENT); + } + + ActionTemplateExpansionFunctionException(ArtifactPrefixConflictException e) { + super(e, Transience.PERSISTENT); + } + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java new file mode 100644 index 0000000000..fa429028b7 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java @@ -0,0 +1,100 @@ +// 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.lib.skyframe; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey; +import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; + +/** + * Value that stores expanded actions from ActionTemplate. + */ +public final class ActionTemplateExpansionValue extends ActionLookupValue { + private final Iterable expandedActions; + + ActionTemplateExpansionValue(Iterable expandedActions) { + super(ImmutableList.copyOf(expandedActions)); + this.expandedActions = ImmutableList.copyOf(expandedActions); + } + + Iterable getExpandedActions() { + return expandedActions; + } + + static SkyKey key(SpawnActionTemplate actionTemplate) { + return SkyKey.create( + SkyFunctions.ACTION_TEMPLATE_EXPANSION, + createActionTemplateExpansionKey(actionTemplate)); + } + + static ActionTemplateExpansionKey createActionTemplateExpansionKey( + SpawnActionTemplate actionTemplate) { + return new ActionTemplateExpansionKey(actionTemplate); + } + + + static final class ActionTemplateExpansionKey extends ActionLookupKey { + private final SpawnActionTemplate actionTemplate; + + ActionTemplateExpansionKey(SpawnActionTemplate actionTemplate) { + Preconditions.checkNotNull( + actionTemplate, + "Passed in action template cannot be null: %s", + actionTemplate); + this.actionTemplate = actionTemplate; + } + + @Override + SkyFunctionName getType() { + return SkyFunctions.ACTION_TEMPLATE_EXPANSION; + } + + + @Override + public Label getLabel() { + return actionTemplate.getOwner().getLabel(); + } + + /** Returns the associated {@link SpawnActionTemplate} */ + SpawnActionTemplate getActionTemplate() { + return actionTemplate; + } + + @Override + public int hashCode() { + return actionTemplate.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (!(obj instanceof ActionTemplateExpansionKey)) { + return false; + } + ActionTemplateExpansionKey other = (ActionTemplateExpansionKey) obj; + return actionTemplate.equals(other.actionTemplate); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index c8022d5e89..6404ed1f68 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java @@ -15,13 +15,17 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.MissingInputFileException; +import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -71,32 +75,84 @@ class ArtifactFunction implements SkyFunction { return null; } - Preconditions.checkState( - actionMetadata instanceof Action, - "%s is not a proper Action object and therefore cannot be executed", - actionMetadata); - Action action = (Action) actionMetadata; - ActionExecutionValue actionValue = + // If the action is an ActionTemplate, we need to expand the ActionTemplate into concrete + // actions, execute those actions in parallel and then aggregate the action execution results. + if (artifact.isTreeArtifact() && actionMetadata instanceof SpawnActionTemplate) { + return createTreeArtifactValueFromActionTemplate( + (SpawnActionTemplate) actionMetadata, artifact, env); + } else { + Preconditions.checkState( + actionMetadata instanceof Action, + "%s is not a proper Action object and therefore cannot be executed", + actionMetadata); + Action action = (Action) actionMetadata; + ActionExecutionValue actionValue = (ActionExecutionValue) env.getValue(ActionExecutionValue.key(action)); - if (actionValue == null) { + if (actionValue == null) { + return null; + } + + if (artifact.isTreeArtifact()) { + // We get a request for the whole tree artifact. We can just return the associated + // TreeArtifactValue. + return Preconditions.checkNotNull(actionValue.getTreeArtifactValue(artifact), artifact); + } else if (isAggregatingValue(action)) { + return createAggregatingValue(artifact, action, + actionValue.getArtifactValue(artifact), env); + } else { + return createSimpleFileArtifactValue(artifact, action, actionValue, env); + } + } + } + + private TreeArtifactValue createTreeArtifactValueFromActionTemplate( + SpawnActionTemplate actionTemplate, Artifact treeArtifact, Environment env) + throws ArtifactFunctionException { + // Request the list of expanded actions from the ActionTemplate. + ActionTemplateExpansionValue expansionValue = (ActionTemplateExpansionValue) env.getValue( + ActionTemplateExpansionValue.key(actionTemplate)); + + // The expanded actions are not yet available. + if (env.valuesMissing()) { return null; } - if (artifact.isTreeArtifact()) { - return actionValue.getTreeArtifactValue(artifact); - } else if (!isAggregatingValue(action)) { - try { - return createSimpleValue(artifact, actionValue); - } catch (IOException e) { - ActionExecutionException ex = new ActionExecutionException(e, action, - /*catastrophe=*/false); - env.getListener().handle(Event.error(ex.getLocation(), ex.getMessage())); - // This is a transient error since we did the work that led to the IOException. - throw new ArtifactFunctionException(ex, Transience.TRANSIENT); + // Execute the expanded actions in parallel. + Iterable expandedActionExecutionKeys = ActionExecutionValue.keys( + expansionValue.getExpandedActions()); + Map expandedActionValueMap = env.getValues(expandedActionExecutionKeys); + + // The execution values of the expanded actions are not yet all available. + if (env.valuesMissing()) { + return null; + } + + // Aggregate the ArtifactValues for individual TreeFileArtifacts into a TreeArtifactValue for + // the parent TreeArtifact. + ImmutableMap.Builder map = ImmutableMap.builder(); + for (Map.Entry entry : expandedActionValueMap.entrySet()) { + SkyKey expandedActionExecutionKey = entry.getKey(); + ActionExecutionValue actionExecutionValue = (ActionExecutionValue) entry.getValue(); + Action expandedAction = (Action) expandedActionExecutionKey.argument(); + + Iterable treeFileArtifacts = findActionOutputsWithMatchingParent( + expandedAction, treeArtifact); + + Preconditions.checkState( + !Iterables.isEmpty(treeFileArtifacts), + "Action %s does not output TreeFileArtifact under %s", + expandedAction, + treeArtifact); + + for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) { + FileArtifactValue value = createSimpleFileArtifactValue( + treeFileArtifact, expandedAction, actionExecutionValue, env); + map.put(treeFileArtifact, value); } - } else { - return createAggregatingValue(artifact, action, actionValue.getArtifactValue(artifact), env); } + + // Return the aggregated TreeArtifactValue. + return TreeArtifactValue.create(map.build()); } private ArtifactValue createSourceValue(Artifact artifact, boolean mandatory, Environment env) @@ -155,9 +211,10 @@ class ArtifactFunction implements SkyFunction { // Non-aggregating artifact -- should contain at most one piece of artifact data. // data may be null if and only if artifact is a middleman artifact. - private ArtifactValue createSimpleValue(Artifact artifact, ActionExecutionValue actionValue) - throws IOException { - ArtifactValue value = actionValue.getArtifactValue(artifact); + private static FileArtifactValue createSimpleFileArtifactValue(Artifact artifact, + Action generatingAction, ActionExecutionValue actionValue, Environment env) + throws ArtifactFunctionException { + FileArtifactValue value = actionValue.getArtifactValue(artifact); if (value != null) { return value; } @@ -168,7 +225,16 @@ class ArtifactFunction implements SkyFunction { "%s %s", artifact, actionValue); Preconditions.checkNotNull(data.getDigest(), "Digest should already have been calculated for %s (%s)", artifact, data); - return FileArtifactValue.create(artifact, data); + + try { + return FileArtifactValue.create(artifact, data); + } catch (IOException e) { + ActionExecutionException ex = new ActionExecutionException(e, generatingAction, + /*catastrophe=*/false); + env.getListener().handle(Event.error(ex.getLocation(), ex.getMessage())); + // This is a transient error since we did the work that led to the IOException. + throw new ArtifactFunctionException(ex, Transience.TRANSIENT); + } } private AggregatingArtifactValue createAggregatingValue(Artifact artifact, @@ -222,8 +288,31 @@ class ArtifactFunction implements SkyFunction { // were created during the first analysis of a configured target. Preconditions.checkNotNull(value, "Owner %s of %s not in graph %s", artifactOwner, artifact, actionLookupKey); - return Preconditions.checkNotNull(value.getGeneratingAction(artifact), - "Value %s does not contain generating action of %s", value, artifact); + + ActionAnalysisMetadata action = value.getGeneratingAction(artifact); + if (artifact.hasParent()) { + // We are trying to resolve the generating action for a TreeFileArtifact. It may not have + // a generating action in the action graph at analysis time. In that case, we get the + // generating action for its parent TreeArtifact, which contains this TreeFileArtifact. + if (action == null) { + action = value.getGeneratingAction(artifact.getParent()); + } + } + + return Preconditions.checkNotNull(action, + "Value %s does not contain generating action of %s", value, artifact); + } + + private static Iterable findActionOutputsWithMatchingParent( + Action action, Artifact treeArtifact) { + ImmutableList.Builder matchingOutputs = ImmutableList.builder(); + for (Artifact output : action.getOutputs()) { + Preconditions.checkState(output.hasParent(), "%s must be a TreeFileArtifact", output); + if (output.getParent().equals(treeArtifact)) { + matchingOutputs.add((TreeFileArtifact) output); + } + } + return matchingOutputs.build(); } private static final class ArtifactFunctionException extends SkyFunctionException { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index e6df1276e5..116d3ba11e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -100,6 +100,8 @@ public final class SkyFunctions { SkyFunctionName.create("REPOSITORY_DIRECTORY"); public static final SkyFunctionName WORKSPACE_AST = SkyFunctionName.create("WORKSPACE_AST"); public static final SkyFunctionName EXTERNAL_PACKAGE = SkyFunctionName.create("EXTERNAL_PACKAGE"); + public static final SkyFunctionName ACTION_TEMPLATE_EXPANSION = + SkyFunctionName.create("ACTION_TEMPLATE_EXPANSION"); public static Predicate isSkyFunction(final SkyFunctionName functionName) { return new Predicate() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 8dfa3d54d2..e33c7e8c8d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -410,6 +410,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, new RecursiveFilesystemTraversalFunction()); map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction()); + map.put(SkyFunctions.ACTION_TEMPLATE_EXPANSION, new ActionTemplateExpansionFunction()); map.putAll(extraSkyFunctions); return map.build(); } -- cgit v1.2.3