Get rid of the last reference to getAttributeMapper by keeping packages which

are referenced by TopLevelTargets when we discard the analysis cache.

PiperOrigin-RevId: 185574670

3 files changed, 113 insertions, 58 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
index 1a597212b8..753ce7091e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
@@ -43,6 +43,9 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.AttributeMap;
+import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
+import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TestSize;
 import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
 import com.google.devtools.build.lib.syntax.Type;
@@ -56,6 +59,7 @@ public final class TargetCompleteEvent
         EventReportingArtifacts,
         BuildEventWithConfiguration {
   private final ConfiguredTarget target;
+  private final Target actualTarget;
   private final NestedSet<Cause> rootCauses;
   private final ImmutableList<BuildEventId> postedAfter;
   private final Iterable<ArtifactsInOutputGroup> outputs;
@@ -64,10 +68,12 @@ public final class TargetCompleteEvent
 
   private TargetCompleteEvent(
       ConfiguredTarget target,
+      Target actualTarget,
       NestedSet<Cause> rootCauses,
       Iterable<ArtifactsInOutputGroup> outputs,
       boolean isTest) {
     this.target = target;
+    this.actualTarget = actualTarget;
     this.rootCauses =
         (rootCauses == null) ? NestedSetBuilder.<Cause>emptySet(Order.STABLE_ORDER) : rootCauses;
 
@@ -100,35 +106,33 @@ public final class TargetCompleteEvent
 
   /** Construct a successful target completion event. */
   public static TargetCompleteEvent successfulBuild(
-      ConfiguredTarget ct, NestedSet<ArtifactsInOutputGroup> outputs) {
-    return new TargetCompleteEvent(ct, null, outputs, false);
+      ConfiguredTarget ct, Target target, NestedSet<ArtifactsInOutputGroup> outputs) {
+    return new TargetCompleteEvent(ct, target, null, outputs, false);
   }
 
   /** Construct a successful target completion event for a target that will be tested. */
-  public static TargetCompleteEvent successfulBuildSchedulingTest(ConfiguredTarget ct) {
-    return new TargetCompleteEvent(ct, null, ImmutableList.<ArtifactsInOutputGroup>of(), true);
+  public static TargetCompleteEvent successfulBuildSchedulingTest(
+      ConfiguredTarget ct, Target target) {
+    return new TargetCompleteEvent(
+        ct, target, null, ImmutableList.<ArtifactsInOutputGroup>of(), true);
   }
 
-
   /**
    * Construct a target completion event for a failed target, with the given non-empty root causes.
    */
-  public static TargetCompleteEvent createFailed(ConfiguredTarget ct, NestedSet<Cause> rootCauses) {
+  public static TargetCompleteEvent createFailed(
+      ConfiguredTarget ct, Target target, NestedSet<Cause> rootCauses) {
     Preconditions.checkArgument(!Iterables.isEmpty(rootCauses));
     return new TargetCompleteEvent(
-        ct, rootCauses, ImmutableList.<ArtifactsInOutputGroup>of(), false);
+        ct, target, rootCauses, ImmutableList.<ArtifactsInOutputGroup>of(), false);
   }
 
-  /**
-   * Returns the target associated with the event.
-   */
+  /** Returns the target associated with the event. */
   public ConfiguredTarget getTarget() {
     return target;
   }
 
-  /**
-   * Determines whether the target has failed or succeeded.
-   */
+  /** Determines whether the target has failed or succeeded. */
   public boolean failed() {
     return !rootCauses.isEmpty();
   }
@@ -160,7 +164,7 @@ public final class TargetCompleteEvent
       // For tests, announce all the test actions that will minimally happen (except for
       // interruption). If after the result of a test action another attempt is necessary,
       // it will be announced with the action that made the new attempt necessary.
-      Label label = target.getTarget().getLabel();
+      Label label = target.getLabel();
       TestProvider.TestParams params = target.getProvider(TestProvider.class).getTestParams();
       for (int run = 0; run < Math.max(params.getRuns(), 1); run++) {
         for (int shard = 0; shard < Math.max(params.getShards(), 1); shard++) {
@@ -192,14 +196,14 @@ public final class TargetCompleteEvent
         BuildEventStreamProtos.TargetComplete.newBuilder();
 
     builder.setSuccess(!failed());
-    builder.setTargetKind(target.getTarget().getTargetKind());
+    builder.setTargetKind(actualTarget.getTargetKind());
     builder.addAllTag(getTags());
     builder.addAllOutputGroup(getOutputFilesByGroup(converters.artifactGroupNamer()));
 
     if (isTest) {
       builder.setTestSize(
           TargetConfiguredEvent.bepTestSize(
-              TestSize.getTestSize(target.getTarget().getAssociatedRule())));
+              TestSize.getTestSize(actualTarget.getAssociatedRule())));
     }
 
     // TODO(aehlig): remove direct reporting of artifacts as soon as clients no longer
@@ -250,7 +254,9 @@ public final class TargetCompleteEvent
     if (!(target instanceof RuleConfiguredTarget)) {
       return ImmutableList.<String>of();
     }
-    AttributeMap attributes = ((RuleConfiguredTarget) target).getAttributeMapper();
+    AttributeMap attributes =
+        ConfiguredAttributeMapper.of(
+            (Rule) actualTarget, ((RuleConfiguredTarget) target).getConfigConditions());
     // Every rule (implicitly) has a "tags" attribute.
     return attributes.get("tags", Type.STRING_LIST);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index f25c559ffb..780d74924f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -74,11 +74,12 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     ArtifactsToBuild getAllArtifactsToBuild(TValue value, TopLevelArtifactContext context);
 
     /** Creates an event reporting an absent input artifact. */
-    Event getRootCauseError(TValue value, Cause rootCause, Environment env);
+    Event getRootCauseError(TValue value, Cause rootCause, Environment env)
+        throws InterruptedException;
 
     /** Creates an error message reporting {@code missingCount} missing input files. */
     MissingInputFileException getMissingFilesException(
-        TValue value, int missingCount, Environment env);
+        TValue value, int missingCount, Environment env) throws InterruptedException;
 
     /**
      * Creates a successful completion value.
@@ -86,11 +87,16 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     TResult createResult(TValue value);
 
     /** Creates a failed completion value. */
-    ExtendedEventHandler.Postable createFailed(TValue value, NestedSet<Cause> rootCauses);
+    ExtendedEventHandler.Postable createFailed(
+        TValue value, NestedSet<Cause> rootCauses, Environment env) throws InterruptedException;
 
     /** Creates a succeeded completion value. */
     ExtendedEventHandler.Postable createSucceeded(
-        SkyKey skyKey, TValue value, TopLevelArtifactContext topLevelArtifactContext);
+        SkyKey skyKey,
+        TValue value,
+        TopLevelArtifactContext topLevelArtifactContext,
+        Environment env)
+        throws InterruptedException;
 
     /**
      * Extracts a tag given the {@link SkyKey}.
@@ -121,41 +127,23 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     }
 
     @Override
-    public Event getRootCauseError(
-        ConfiguredTargetValue ctValue, Cause rootCause, Environment env) {
-      Target target = null;
-      try {
-        target =
-            ((PackageValue)
-                    env.getValue(
-                        PackageValue.key(
-                            ctValue.getConfiguredTarget().getLabel().getPackageIdentifier())))
-                .getPackage()
-                .getTarget(ctValue.getConfiguredTarget().getLabel().getName());
-      } catch (NoSuchTargetException | InterruptedException e) {
-        throw new IllegalStateException("Failed to retrieve target to get a root cause error.");
-      }
+    public Event getRootCauseError(ConfiguredTargetValue ctValue, Cause rootCause, Environment env)
+        throws InterruptedException {
+      Target target = getTargetFromConfiguredTarget(ctValue.getConfiguredTarget(), env);
       return Event.error(
-          target.getLocation(),
+          target == null ? null : target.getLocation(),
           String.format(
               "%s: missing input file '%s'", ctValue.getConfiguredTarget().getLabel(), rootCause));
     }
 
     @Override
+    @Nullable
     public MissingInputFileException getMissingFilesException(
-        ConfiguredTargetValue value, int missingCount, Environment env) {
-      Target target = null;
-      try {
-        target =
-            ((PackageValue)
-                    env.getValue(
-                        PackageValue.key(
-                            value.getConfiguredTarget().getLabel().getPackageIdentifier())))
-                .getPackage()
-                .getTarget(value.getConfiguredTarget().getLabel().getName());
-      } catch (NoSuchTargetException | InterruptedException e) {
-        throw new IllegalStateException(
-            "Failed to retrieve target to create MissingFilesException.");
+        ConfiguredTargetValue value, int missingCount, Environment env)
+        throws InterruptedException {
+      Target target = getTargetFromConfiguredTarget(value.getConfiguredTarget(), env);
+      if (target == null) {
+        return null;
       }
       return new MissingInputFileException(
           target.getLocation() + " " + missingCount + " input file(s) do not exist",
@@ -168,9 +156,16 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     }
 
     @Override
+    @Nullable
     public ExtendedEventHandler.Postable createFailed(
-        ConfiguredTargetValue value, NestedSet<Cause> rootCauses) {
-      return TargetCompleteEvent.createFailed(value.getConfiguredTarget(), rootCauses);
+        ConfiguredTargetValue value, NestedSet<Cause> rootCauses, Environment env)
+        throws InterruptedException {
+      Target actualTarget = getTargetFromConfiguredTarget(value.getConfiguredTarget(), env);
+      if (actualTarget == null) {
+        return null;
+      }
+      return TargetCompleteEvent.createFailed(
+          value.getConfiguredTarget(), actualTarget, rootCauses);
     }
 
     @Override
@@ -180,20 +175,44 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     }
 
     @Override
+    @Nullable
     public ExtendedEventHandler.Postable createSucceeded(
         SkyKey skyKey,
         ConfiguredTargetValue value,
-        TopLevelArtifactContext topLevelArtifactContext) {
+        TopLevelArtifactContext topLevelArtifactContext,
+        Environment env)
+        throws InterruptedException {
       ConfiguredTarget target = value.getConfiguredTarget();
+      Target actualTarget = getTargetFromConfiguredTarget(target, env);
+      if (target == null) {
+        return null;
+      }
       if (((TargetCompletionKey) skyKey.argument()).willTest()) {
-        return TargetCompleteEvent.successfulBuildSchedulingTest(target);
+        return TargetCompleteEvent.successfulBuildSchedulingTest(target, actualTarget);
       } else {
         ArtifactsToBuild artifactsToBuild =
             TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext);
         return TargetCompleteEvent.successfulBuild(
-            target, artifactsToBuild.getAllArtifactsByOutputGroup());
+            target, actualTarget, artifactsToBuild.getAllArtifactsByOutputGroup());
       }
     }
+
+    @Nullable
+    private Target getTargetFromConfiguredTarget(ConfiguredTarget ct, Environment env)
+        throws InterruptedException {
+      Target target = null;
+      try {
+        PackageValue packageValue =
+            (PackageValue) env.getValue(PackageValue.key(ct.getLabel().getPackageIdentifier()));
+        if (packageValue != null) {
+          target = packageValue.getPackage().getTarget(ct.getLabel().getName());
+        }
+      } catch (NoSuchTargetException e) {
+        throw new IllegalStateException(
+            "Failed to retrieve target to create MissingFilesException.", e);
+      }
+      return target;
+    }
   }
 
   private static class AspectCompletor implements Completor<AspectValue, AspectCompletionValue> {
@@ -247,7 +266,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
 
     @Override
     public ExtendedEventHandler.Postable createFailed(
-        AspectValue value, NestedSet<Cause> rootCauses) {
+        AspectValue value, NestedSet<Cause> rootCauses, Environment env) {
       return AspectCompleteEvent.createFailed(value, rootCauses);
     }
 
@@ -258,7 +277,10 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
 
     @Override
     public ExtendedEventHandler.Postable createSucceeded(
-        SkyKey skyKey, AspectValue value, TopLevelArtifactContext topLevelArtifactContext) {
+        SkyKey skyKey,
+        AspectValue value,
+        TopLevelArtifactContext topLevelArtifactContext,
+        Environment env) {
       ArtifactsToBuild artifacts =
           TopLevelArtifactHelper.getAllArtifactsToBuild(value, topLevelArtifactContext);
       return AspectCompleteEvent.createSuccessful(value, artifacts);
@@ -325,11 +347,18 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
 
     if (missingCount > 0) {
       missingInputException = completor.getMissingFilesException(value, missingCount, env);
+      if (missingInputException == null) {
+        return null;
+      }
     }
 
     NestedSet<Cause> rootCauses = rootCausesBuilder.build();
     if (!rootCauses.isEmpty()) {
-      env.getListener().post(completor.createFailed(value, rootCauses));
+      ExtendedEventHandler.Postable postable = completor.createFailed(value, rootCauses, env);
+      if (postable == null) {
+        return null;
+      }
+      env.getListener().post(postable);
       if (firstActionExecutionException != null) {
         throw new CompletionFunctionException(firstActionExecutionException);
       } else {
@@ -343,7 +372,12 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     if (env.valuesMissing()) {
       return null;
     }
-    env.getListener().post(completor.createSucceeded(skyKey, value, topLevelContext));
+    ExtendedEventHandler.Postable postable =
+        completor.createSucceeded(skyKey, value, topLevelContext, env);
+    if (postable == null) {
+      return null;
+    }
+    env.getListener().post(postable);
     return completor.createResult(value);
   }
 
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 20b05f0d46..3f271b30c8 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
@@ -18,6 +18,7 @@ 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;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -645,6 +646,15 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
       Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
     topLevelTargets = ImmutableSet.copyOf(topLevelTargets);
     topLevelAspects = ImmutableSet.copyOf(topLevelAspects);
+    // This is to prevent throwing away Packages we may need during execution.
+    ImmutableSet.Builder<PackageIdentifier> packageSetBuilder = ImmutableSet.builder();
+    packageSetBuilder.addAll(
+        Collections2.transform(
+            topLevelTargets, (target) -> target.getLabel().getPackageIdentifier()));
+    packageSetBuilder.addAll(
+        Collections2.transform(
+            topLevelAspects, (aspect) -> aspect.getLabel().getPackageIdentifier()));
+    ImmutableSet<PackageIdentifier> topLevelPackages = packageSetBuilder.build();
     try (AutoProfiler p = AutoProfiler.logged("discarding analysis cache", logger)) {
       lastAnalysisDiscarded = true;
       Iterator<? extends Map.Entry<SkyKey, ? extends NodeEntry>> it =
@@ -657,6 +667,11 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         }
         SkyKey key = keyAndEntry.getKey();
         SkyFunctionName functionName = key.functionName();
+        // Keep packages for top-level targets and aspects in memory to get the target from later.
+        if (functionName.equals(SkyFunctions.PACKAGE)
+            && topLevelPackages.contains((key.argument()))) {
+          continue;
+        }
         if (!tracksStateForIncrementality() && LOADING_TYPES.contains(functionName)) {
           it.remove();
           continue;