commit c902691d8c8944c2cc8b74e7757bdda5af7c4287
author janakr <janakr@google.com> Wed Apr 14 10:28:23 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Apr 14 10:29:44 2021 -0700
tree e2d0b851191f5543a89529340f6af34742189181
parent 453848148ff48961ddb7894761c2c22414a9c429

Refactor the ActionLookupValue/ConfiguredObjectValue hierarchy to reflect the reality that some configured targets will never have associated actions (all non-rule configured targets, including input/output files, aliases, and package groups).

This allows us to more explicitly track the different categories of configured targets for metrics. Because non-rule configured targets can depend on rule configured targets (an output file depends on its generating rule, an alias depends on a target), the traversal of the analysis graph that powers these metrics must still visit these non-rule configured targets, allowing them to be tracked with little additional overhead.

As a small optimization, we can delete from the Skyframe graph any nodes corresponding to non-rule configured targets if dropping the analysis phase.

PiperOrigin-RevId: 368457602

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
index 3826ef9..e1c96d2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java
@@ -13,13 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
-import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
-import com.google.devtools.build.lib.actions.BasicActionLookupValue;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
@@ -36,10 +31,7 @@
 @ThreadSafe
 // Reached via OutputFileConfiguredTarget.
 @AutoCodec(explicitlyAllowClass = RuleConfiguredTarget.class)
-@VisibleForTesting
-public final class NonRuleConfiguredTargetValue extends BasicActionLookupValue
-    implements ConfiguredTargetValue {
-
+public final class NonRuleConfiguredTargetValue implements ConfiguredTargetValue {
   // These variables are only non-final because they may be clear()ed to save memory.
   // configuredTarget is null only after it is cleared.
   @Nullable private ConfiguredTarget configuredTarget;
@@ -50,20 +42,15 @@
   @AutoCodec.Instantiator
   @VisibleForSerialization
   NonRuleConfiguredTargetValue(
-      ImmutableList<ActionAnalysisMetadata> actions,
       ConfiguredTarget configuredTarget) {
-    super(actions);
-    this.configuredTarget = configuredTarget;
     // Transitive packages are not serialized.
-    this.transitivePackagesForPackageRootResolution = null;
+    this(configuredTarget, null);
   }
 
   NonRuleConfiguredTargetValue(
       ConfiguredTarget configuredTarget,
-      GeneratingActions generatingActions,
       @Nullable NestedSet<Package> transitivePackagesForPackageRootResolution) {
-    super(generatingActions);
-    this.configuredTarget = Preconditions.checkNotNull(configuredTarget, generatingActions);
+    this.configuredTarget = Preconditions.checkNotNull(configuredTarget);
     this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution;
   }
 
@@ -74,12 +61,6 @@
   }
 
   @Override
-  public ImmutableList<ActionAnalysisMetadata> getActions() {
-    Preconditions.checkNotNull(configuredTarget, this);
-    return actions;
-  }
-
-  @Override
   public NestedSet<Package> getTransitivePackagesForPackageRootResolution() {
     return Preconditions.checkNotNull(transitivePackagesForPackageRootResolution);
   }
@@ -95,11 +76,6 @@
 
   @Override
   public String toString() {
-    return getStringHelper().add("configuredTarget", configuredTarget).toString();
-  }
-
-  @Override
-  public SourceArtifact getSourceArtifact() {
-    return configuredTarget == null ? null : configuredTarget.getSourceArtifact();
+    return MoreObjects.toStringHelper(this).add("configuredTarget", configuredTarget).toString();
   }
 }