Skip to content

Commit

Permalink
Get ActionAnalysisMetadata in a simpler, more efficient manner.
Browse files Browse the repository at this point in the history
Instead of checking whether it's an action or a template and calling the corresponding method, when it can be either, simply bypass checks and get it directly from the list.

The actions list itself used to not be exposed, so isActionTemplate was necessary to determine which method to call (getAction or getActionTemplate).

Also prevent boxing of ints in the Preconditions calls.

PiperOrigin-RevId: 261174117
  • Loading branch information
Googler authored and copybara-github committed Aug 1, 2019
1 parent 90a55a9 commit 2714b88
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.cmdline.Label;
Expand All @@ -39,29 +38,25 @@ protected ActionLookupValue(@Nullable BigInteger nonceVersion) {
* 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) {
Preconditions.checkState(index < getActions().size(), "Bad index: %s %s", index, this);
ActionAnalysisMetadata result = getActions().get(index);
Preconditions.checkState(result instanceof Action, "Not action: %s %s %s", result, index, this);
// Avoid Preconditions.checkState which would box the int arg.
if (!(result instanceof Action)) {
throw new IllegalStateException(String.format("Not action: %s %s %s", result, index, this));
}
return (Action) result;
}

public ActionTemplate<?> getActionTemplate(int index) {
ActionAnalysisMetadata result = getActions().get(index);
Preconditions.checkState(
result instanceof ActionTemplate, "Not action template: %s %s %s", result, index, this);
// Avoid Preconditions.checkState which would box the int arg.
if (!(result instanceof ActionTemplate)) {
throw new IllegalStateException(
String.format("Not action template: %s %s %s", result, index, this));
}
return (ActionTemplate<?>) result;
}

/**
* Returns if the action at {@code index} is an {@link ActionTemplate} so that tree artifacts can
* take the proper action.
*/
public boolean isActionTemplate(int index) {
return getActions().get(index) instanceof ActionTemplate;
}

/**
* Returns the number of {@link Action} objects present in this value.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,10 +535,7 @@ public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
"Missing generating action for %s (%s)", artifact, generatingActionKey);
return null;
}
int actionIndex = generatingActionKey.getActionIndex();
return val.isActionTemplate(actionIndex)
? val.getActionTemplate(actionIndex)
: val.getAction(actionIndex);
return val.getActions().get(generatingActionKey.getActionIndex());
}
};
return new AnalysisResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
Expand Down Expand Up @@ -454,7 +455,8 @@ static ArtifactDependencies discoverDependencies(

boolean isTemplateActionForTreeArtifact() {
return artifact.isTreeArtifact()
&& actionLookupValue.isActionTemplate(artifact.getGeneratingActionKey().getActionIndex());
&& actionLookupValue.getActions().get(artifact.getGeneratingActionKey().getActionIndex())
instanceof ActionTemplate;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2519,10 +2519,7 @@ private ActionAnalysisMetadata getGeneratingAction(
return null;
}
ActionLookupValue actionLookupValue = result.get(generatingActionKey.getActionLookupKey());
int actionIndex = generatingActionKey.getActionIndex();
return actionLookupValue.isActionTemplate(actionIndex)
? actionLookupValue.getActionTemplate(actionIndex)
: actionLookupValue.getAction(actionIndex);
return actionLookupValue.getActions().get(generatingActionKey.getActionIndex());
}
}

Expand Down

0 comments on commit 2714b88

Please sign in to comment.