commit cb314a2e36031c8a5f1dd26bb3b94f1b8f1cb901
author janakr <janakr@google.com> Thu Feb 15 10:33:53 2018 -0800
committer Copybara-Service <copybara-piper@google.com> Thu Feb 15 10:35:15 2018 -0800
tree e04b828849bd9b5be4d4bc1dde67b46cf46de9d7
parent 10b1a5fc3ade9c5f12078cd616218f9ef45ef13c

Stop storing ActionTemplate in a SkyKey: it's too heavyweight. Use the same mechanism as for normal actions, have the ActionTemplateExpansionFunction look the template up when needed.

PiperOrigin-RevId: 185861672

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java

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 6e6e432..5cef0fc 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
@@ -29,12 +29,10 @@
 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.ActionTemplate;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -84,26 +82,21 @@
 
     // 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()) {
-      ActionAnalysisMetadata actionMetadata =
-          actionLookupValue.getIfPresentAndNotAction(actionIndex);
-      if (actionMetadata instanceof ActionTemplate) {
-        // Create the directory structures for the output TreeArtifact first.
-        try {
-          FileSystemUtils.createDirectoryAndParents(artifact.getPath());
-        } catch (IOException e) {
-          env.getListener()
-              .handle(
-                  Event.error(
-                      String.format(
-                          "Failed to create output directory for TreeArtifact %s: %s",
-                          artifact, e.getMessage())));
-          throw new ArtifactFunctionException(e, Transience.TRANSIENT);
-        }
-
-        return createTreeArtifactValueFromActionTemplate(
-            (ActionTemplate<?>) actionMetadata, artifact, env);
+    if (artifact.isTreeArtifact() && actionLookupValue.isActionTemplate(actionIndex)) {
+      // Create the directory structures for the output TreeArtifact first.
+      try {
+        artifact.getPath().createDirectoryAndParents();
+      } catch (IOException e) {
+        env.getListener()
+            .handle(
+                Event.error(
+                    String.format(
+                        "Failed to create output directory for TreeArtifact %s: %s",
+                        artifact, e.getMessage())));
+        throw new ArtifactFunctionException(e, Transience.TRANSIENT);
       }
+
+      return createTreeArtifactValueFromActionKey(actionLookupKey, actionIndex, artifact, env);
     }
     ActionExecutionValue actionValue =
         (ActionExecutionValue) env.getValue(ActionExecutionValue.key(actionLookupKey, actionIndex));
@@ -133,12 +126,15 @@
     return createSimpleFileArtifactValue(artifact, actionValue);
   }
 
-  private static TreeArtifactValue createTreeArtifactValueFromActionTemplate(
-      final ActionTemplate<?> actionTemplate, final Artifact treeArtifact, Environment env)
-          throws InterruptedException {
+  private static TreeArtifactValue createTreeArtifactValueFromActionKey(
+      ActionLookupKey actionLookupKey,
+      int actionIndex,
+      final Artifact treeArtifact,
+      Environment env)
+      throws InterruptedException {
     // Request the list of expanded actions from the ActionTemplate.
     ActionTemplateExpansionValue.ActionTemplateExpansionKey templateKey =
-        ActionTemplateExpansionValue.key(actionTemplate);
+        ActionTemplateExpansionValue.key(actionLookupKey, actionIndex);
     ActionTemplateExpansionValue expansionValue =
         (ActionTemplateExpansionValue) env.getValue(templateKey);
 
@@ -166,9 +162,10 @@
           (ActionExecutionValue)
               Preconditions.checkNotNull(
                   expandedActionValueMap.get(expandedActionExecutionKeys.get(i)),
-                  "Missing tree value: %s %s %s %s",
+                  "Missing tree value: %s %s %s %s %s",
                   treeArtifact,
-                  actionTemplate,
+                  actionLookupKey,
+                  actionIndex,
                   expansionValue,
                   expandedActionValueMap);
       Iterable<TreeFileArtifact> treeFileArtifacts =
@@ -180,11 +177,12 @@
                     public boolean apply(Artifact artifact) {
                       Preconditions.checkState(
                           artifact.hasParent(),
-                          "No parent: %s %s %s %s",
+                          "No parent: %s %s %s %s %s",
                           artifact,
                           treeArtifact,
                           actionExecutionValue,
-                          actionTemplate);
+                          actionLookupKey,
+                          actionIndex);
                       return artifact.getParent().equals(treeArtifact);
                     }
                   }),