diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/actions')
7 files changed, 371 insertions, 21 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java index 101cc3932a..ad2148a089 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java @@ -439,6 +439,11 @@ public abstract class AbstractAction implements Action, SkylarkValue { return MiddlemanType.NORMAL; } + @Override + public boolean canRemoveAfterExecution() { + return true; + } + /** * If the action might create directories as outputs this method must be called. */ diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java index d490e6a258..33b0b16658 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Action.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java @@ -185,6 +185,8 @@ public interface Action extends ActionExecutionMetadata, Describable { */ boolean showsOutputUnconditionally(); + boolean canRemoveAfterExecution(); + /** * Returns true if an {@link com.google.devtools.build.lib.rules.extra.ExtraAction} action can be * attached to this action. If not, extra actions should not be attached to this action. diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java index 497f96f7b8..d4621ea827 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCompletionEvent.java @@ -20,10 +20,13 @@ public final class ActionCompletionEvent { private final long relativeActionStartTime; private final Action action; + private final ActionLookupData actionLookupData; - public ActionCompletionEvent(long relativeActionStartTime, Action action) { + public ActionCompletionEvent( + long relativeActionStartTime, Action action, ActionLookupData actionLookupData) { this.relativeActionStartTime = relativeActionStartTime; this.action = action; + this.actionLookupData = actionLookupData; } /** @@ -36,4 +39,8 @@ public final class ActionCompletionEvent { public long getRelativeActionStartTime() { return relativeActionStartTime; } + + public ActionLookupData getActionLookupData() { + return actionLookupData; + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java new file mode 100644 index 0000000000..fea6d6d357 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java @@ -0,0 +1,74 @@ +// Copyright 2017 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.actions; + +import com.google.common.base.MoreObjects; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.skyframe.SkyKey; + +/** Data that uniquely identifies an action. */ +public class ActionLookupData { + private final SkyKey actionLookupNode; + private final int actionIndex; + + public ActionLookupData(SkyKey actionLookupNode, int actionIndex) { + Preconditions.checkState( + actionLookupNode.argument() instanceof ActionLookupValue.ActionLookupKey, actionLookupNode); + this.actionLookupNode = actionLookupNode; + this.actionIndex = actionIndex; + } + + public SkyKey getActionLookupNode() { + return actionLookupNode; + } + + /** + * Index of the action in question in the node keyed by {@link #getActionLookupNode}. Should be + * passed to {@link ActionLookupValue#getAction}. + */ + public int getActionIndex() { + return actionIndex; + } + + public Label getLabelForErrors() { + return ((ActionLookupValue.ActionLookupKey) actionLookupNode.argument()).getLabel(); + } + + @Override + public int hashCode() { + return 37 * actionLookupNode.hashCode() + actionIndex; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ActionLookupData)) { + return false; + } + ActionLookupData that = (ActionLookupData) obj; + return this.actionIndex == that.actionIndex + && this.actionLookupNode.equals(that.actionLookupNode); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("actionLookupNode", actionLookupNode) + .add("actionIndex", actionIndex) + .toString(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java new file mode 100644 index 0000000000..b8c105f7b5 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java @@ -0,0 +1,232 @@ +// Copyright 2014 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.actions; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; +import com.google.common.base.MoreObjects; +import com.google.common.base.MoreObjects.ToStringHelper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + +/** + * Base class for all values which can provide the generating action of an artifact. The primary + * instance of such lookup values is {@code ConfiguredTargetValue}. Values that hold the generating + * actions of target completion values and build info artifacts also fall into this category. + */ +public class ActionLookupValue implements SkyValue { + protected final List<ActionAnalysisMetadata> actions; + private final ImmutableMap<Artifact, Integer> generatingActionIndex; + + private static Actions.GeneratingActions filterSharedActionsAndThrowRuntimeIfConflict( + List<ActionAnalysisMetadata> actions) { + try { + return Actions.filterSharedActionsAndThrowActionConflict(actions); + } catch (ActionConflictException e) { + // Programming bug. + throw new IllegalStateException(e); + } + } + + @VisibleForTesting + public ActionLookupValue( + List<ActionAnalysisMetadata> actions, boolean removeActionsAfterEvaluation) { + this(filterSharedActionsAndThrowRuntimeIfConflict(actions), removeActionsAfterEvaluation); + } + + protected ActionLookupValue(ActionAnalysisMetadata action, boolean removeActionAfterEvaluation) { + this(ImmutableList.of(action), removeActionAfterEvaluation); + } + + protected ActionLookupValue( + Actions.GeneratingActions generatingActions, boolean removeActionsAfterEvaluation) { + if (removeActionsAfterEvaluation) { + this.actions = new ArrayList<>(generatingActions.getActions()); + } else { + this.actions = ImmutableList.copyOf(generatingActions.getActions()); + } + this.generatingActionIndex = generatingActions.getGeneratingActionIndex(); + } + + /** + * Returns the action that generates {@code artifact}, if known to this value, or null. This + * method should be avoided. Call it only when the action is really needed, and it is known to be + * present, either because the execution phase has not started, or because {@link + * Action#canRemoveAfterExecution} is known to be false for the action being requested. + */ + @Nullable + public ActionAnalysisMetadata getGeneratingActionDangerousReadJavadoc(Artifact artifact) { + Integer actionIndex = getGeneratingActionIndex(artifact); + if (actionIndex == null) { + return null; + } + return getActionAnalysisMetadata(actionIndex); + } + + /** + * Returns the index of the action that generates {@code artifact} in this value, or null if this + * value does not have a generating action for this artifact. The index together with the key for + * this {@link ActionLookupValue} uniquely identifies the action. + * + * <p>Unlike {@link #getAction}, this may be called after action execution. + */ + @Nullable + public Integer getGeneratingActionIndex(Artifact artifact) { + return generatingActionIndex.get(artifact); + } + + /** + * Returns the {@link Action} with index {@code index} in this value. Never null. Should only be + * called during action execution by {@code ArtifactFunction} and {@code ActionExecutionFunction} + * -- after an action has executed, calling this with its index may crash. + */ + @SuppressWarnings("unchecked") // We test to make sure it's an Action. + public Action getAction(int index) { + ActionAnalysisMetadata result = getActionAnalysisMetadata(index); + Preconditions.checkState(result instanceof Action, "Not action: %s %s %s", result, index, this); + return (Action) result; + } + + private ActionAnalysisMetadata getActionAnalysisMetadata(int index) { + return Preconditions.checkNotNull(actions.get(index), "null action: %s %s", index, this); + } + + /** + * Returns the {@link ActionAnalysisMetadata} at index {@code index} if it is present and + * <i>not</i> an {@link Action}. Tree artifacts need their {@code ActionTemplate}s in order to + * generate the correct actions, but in general most actions are not needed after they are + * executed and may not even be available. + */ + public ActionAnalysisMetadata getIfPresentAndNotAction(int index) { + ActionAnalysisMetadata actionAnalysisMetadata = actions.get(index); + if (!(actionAnalysisMetadata instanceof Action)) { + return actionAnalysisMetadata; + } + return null; + } + + /** To be used only when checking consistency of the action graph -- not by other values. */ + public Map<Artifact, ActionAnalysisMetadata> getMapForConsistencyCheck() { + return getMapForConsistencyCheck(generatingActionIndex, actions); + } + + protected ToStringHelper getStringHelper() { + return MoreObjects.toStringHelper(this) + .add("actions", actions) + .add("generatingActionIndex", generatingActionIndex); + } + + @Override + public String toString() { + return getStringHelper().toString(); + } + + @VisibleForTesting + public static SkyKey key(ActionLookupKey ownerKey) { + return ownerKey.getSkyKey(); + } + + public int getNumActions() { + return actions.size(); + } + + public static Map<Artifact, ActionAnalysisMetadata> getMapForConsistencyCheck( + Map<Artifact, Integer> generatingActionIndex, + final List<? extends ActionAnalysisMetadata> actions) { + return Maps.transformValues( + generatingActionIndex, + new Function<Integer, ActionAnalysisMetadata>() { + @Override + public ActionAnalysisMetadata apply(Integer index) { + return actions.get(index); + } + }); + } + + /** + * If this object was initialized with {@code removeActionsAfterEvaluation} and {@link + * Action#canRemoveAfterExecution()} is true for {@code action}, then remove this action from this + * object's index as a memory-saving measure. The {@code artifact -> index} mapping remains + * intact, so this action's execution value can still be addressed by its inputs. + */ + @ThreadSafe + public void actionEvaluated(int actionIndex, Action action) { + if (!action.canRemoveAfterExecution()) { + return; + } + if (actions instanceof ArrayList) { + // This method may concurrently mutate an ArrayList, which is unsafe on its face. However, + // ArrayList mutation on different indices that does not affect the size of the ArrayList is + // safe, and that is what does this code does. + ArrayList<ActionAnalysisMetadata> actionArrayList = + (ArrayList<ActionAnalysisMetadata>) actions; + ActionAnalysisMetadata oldAction = actionArrayList.set(actionIndex, null); + Preconditions.checkState( + action.equals(oldAction), "Not same: %s %s %s %s", action, oldAction, this, actionIndex); + } + } + + /** + * ArtifactOwner is not a SkyKey, but we wish to convert any ArtifactOwner into a SkyKey as simply + * as possible. To that end, all subclasses of ActionLookupValue "own" artifacts with + * ArtifactOwners that are subclasses of ActionLookupKey. This allows callers to easily find the + * value key, while remaining agnostic to what ActionLookupValues actually exist. + * + * <p>The methods of this class should only be called by {@link ActionLookupValue#key}. + */ + public abstract static class ActionLookupKey implements ArtifactOwner { + @Override + public Label getLabel() { + return null; + } + + /** + * Subclasses must override this to specify their specific value type, unless they override + * {@link #getSkyKey}, in which case they are free not to implement this method. + */ + protected abstract SkyFunctionName getType(); + + protected SkyKey getSkyKeyInternal() { + return SkyKey.create(getType(), this); + } + + /** + * Prefer {@link ActionLookupValue#key} to calling this method directly. + * + * <p>Subclasses may override {@link #getSkyKeyInternal} if the {@link SkyKey} argument should + * not be this {@link ActionLookupKey} itself. + */ + public final SkyKey getSkyKey() { + SkyKey result = getSkyKeyInternal(); + Preconditions.checkState( + result.argument() instanceof ActionLookupKey, + "Not ActionLookupKey for %s: %s", + this, + result); + return result; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/Actions.java b/src/main/java/com/google/devtools/build/lib/actions/Actions.java index b8bd981156..8a49af4bad 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Actions.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Actions.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.actions; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.escape.Escaper; @@ -23,14 +24,13 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.SortedMap; import java.util.TreeMap; - import javax.annotation.Nullable; /** @@ -85,12 +85,12 @@ public final class Actions { * artifact. Shared actions are tolerated. See {@link #canBeShared} for details. * * @param actions a list of actions to check for action conflicts - * @return a map between generated artifacts and their associated generating actions. If there is - * more than one action generating the same output artifact, only one action is chosen. + * @return a structure giving the mapping between artifacts and generating actions, with a level + * of indirection. * @throws ActionConflictException iff there are two actions generate the same output */ - public static ImmutableMap<Artifact, ActionAnalysisMetadata> findAndThrowActionConflict( - Iterable<ActionAnalysisMetadata> actions) throws ActionConflictException { + public static GeneratingActions findAndThrowActionConflict(List<ActionAnalysisMetadata> actions) + throws ActionConflictException { return Actions.maybeFilterSharedActionsAndThrowIfConflict( actions, /*allowSharedAction=*/ false); } @@ -100,34 +100,34 @@ public final class Actions { * artifact. Shared actions are tolerated. See {@link #canBeShared} for details. * * @param actions a list of actions to check for action conflicts - * @return a map between generated artifacts and their associated generating actions. If there is - * more than one action generating the same output artifact, only one action is chosen. + * @return a structure giving the mapping between artifacts and generating actions, with a level + * of indirection. * @throws ActionConflictException iff there are two unshareable actions generating the same * output */ - public static ImmutableMap<Artifact, ActionAnalysisMetadata> - filterSharedActionsAndThrowActionConflict( - Iterable<? extends ActionAnalysisMetadata> actions) throws ActionConflictException { + public static GeneratingActions filterSharedActionsAndThrowActionConflict( + List<ActionAnalysisMetadata> actions) throws ActionConflictException { return Actions.maybeFilterSharedActionsAndThrowIfConflict( actions, /*allowSharedAction=*/ true); } - private static ImmutableMap<Artifact, ActionAnalysisMetadata> - maybeFilterSharedActionsAndThrowIfConflict( - Iterable<? extends ActionAnalysisMetadata> actions, boolean allowSharedAction) + private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict( + List<ActionAnalysisMetadata> actions, boolean allowSharedAction) throws ActionConflictException { - Map<Artifact, ActionAnalysisMetadata> generatingActions = new HashMap<>(); + Map<Artifact, Integer> generatingActions = new HashMap<>(); + int actionIndex = 0; for (ActionAnalysisMetadata action : actions) { for (Artifact artifact : action.getOutputs()) { - ActionAnalysisMetadata previousAction = generatingActions.put(artifact, action); - if (previousAction != null && previousAction != action) { - if (!allowSharedAction || !Actions.canBeShared(previousAction, action)) { - throw new ActionConflictException(artifact, previousAction, action); + Integer previousIndex = generatingActions.put(artifact, actionIndex); + if (previousIndex != null && previousIndex != actionIndex) { + if (!allowSharedAction || !Actions.canBeShared(actions.get(previousIndex), action)) { + throw new ActionConflictException(artifact, actions.get(previousIndex), action); } } } + actionIndex++; } - return ImmutableMap.copyOf(generatingActions); + return new GeneratingActions(actions, ImmutableMap.copyOf(generatingActions)); } /** @@ -252,4 +252,27 @@ public final class Actions { return generatingActions.get(artifact); } } + + /** Container class for actions and the artifacts they generate. */ + @VisibleForTesting + public static class GeneratingActions { + private final List<ActionAnalysisMetadata> actions; + private final ImmutableMap<Artifact, Integer> generatingActionIndex; + + @VisibleForTesting + public GeneratingActions( + List<ActionAnalysisMetadata> actions, + ImmutableMap<Artifact, Integer> generatingActionIndex) { + this.actions = actions; + this.generatingActionIndex = generatingActionIndex; + } + + public ImmutableMap<Artifact, Integer> getGeneratingActionIndex() { + return generatingActionIndex; + } + + public List<ActionAnalysisMetadata> getActions() { + return actions; + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java index a42d5b958d..7474c7c13d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java @@ -73,6 +73,13 @@ public final class MiddlemanAction extends AbstractAction { } @Override + public boolean canRemoveAfterExecution() { + // Aggregating middleman actions' inputs are needed by their output artifacts, and the type of + // the middleman isn't known without having the action, so we just make sure it stays. + return false; + } + + @Override protected String getRawProgressMessage() { return null; // users don't really want to know about Middlemen. } |