Convert ActionLookupKey implementations to directly implement SkyKey, removing the layer of indirection of getting SkyKey out of ActionLookupKey, which uses more memory for no reason.

PiperOrigin-RevId: 181658615

30 files changed, 244 insertions, 209 deletions

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
index ffe650a19b..f1d91ba376 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -23,8 +23,6 @@ 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.skyframe.LegacySkyKey;
-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;
@@ -151,11 +149,6 @@ public class ActionLookupValue implements SkyValue {
     return getStringHelper().toString();
   }
 
-  @VisibleForTesting
-  public static SkyKey key(ActionLookupKey ownerKey) {
-    return ownerKey.getSkyKey();
-  }
-
   public int getNumActions() {
     return actions.size();
   }
@@ -190,43 +183,14 @@ public class ActionLookupValue implements SkyValue {
   }
 
   /**
-   * 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}.
+   * All subclasses of ActionLookupValue "own" artifacts with {@link ArtifactOwner}s that are
+   * subclasses of ActionLookupKey. This allows callers to easily find the value key, while
+   * remaining agnostic to what ActionLookupValues actually exist.
    */
-  public abstract static class ActionLookupKey implements ArtifactOwner {
+  public abstract static class ActionLookupKey implements ArtifactOwner, SkyKey {
     @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 LegacySkyKey.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/analysis/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
index d5e1f00998..35eeae8959 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
@@ -156,7 +156,7 @@ public final class AspectResolver {
         dependentAspects.build(),
         aspectDeps.getAspect(),
         dep.getAspectConfiguration(aspectDeps.getAspect()));
-    result.put(aspectKey.getAspectDescriptor(), aspectKey.getSkyKey());
+    result.put(aspectKey.getAspectDescriptor(), aspectKey);
     return aspectKey;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 660f69b413..4b58054fda 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -490,13 +490,15 @@ public class BuildView {
       eventBus.post(new TargetConfiguredEvent(target, byLabel.get(target.getLabel())));
     }
 
-    List<ConfiguredTargetKey> topLevelCtKeys = Lists.transform(topLevelTargetsWithConfigs,
-        new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
-          @Override
-          public ConfiguredTargetKey apply(TargetAndConfiguration node) {
-            return new ConfiguredTargetKey(node.getLabel(), node.getConfiguration());
-          }
-        });
+    List<ConfiguredTargetKey> topLevelCtKeys =
+        Lists.transform(
+            topLevelTargetsWithConfigs,
+            new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
+              @Override
+              public ConfiguredTargetKey apply(TargetAndConfiguration node) {
+                return ConfiguredTargetKey.of(node.getLabel(), node.getConfiguration());
+              }
+            });
 
     Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
         ArrayListMultimap.create();
@@ -646,13 +648,14 @@ public class BuildView {
     Iterables.addAll(artifactsToBuild, baselineCoverageArtifacts);
     if (coverageReportActionFactory != null) {
       CoverageReportActionsWrapper actionsWrapper;
-      actionsWrapper = coverageReportActionFactory.createCoverageReportActionsWrapper(
-          eventHandler,
-          directories,
-          allTargetsToTest,
-          baselineCoverageArtifacts,
-          getArtifactFactory(),
-          CoverageReportValue.ARTIFACT_OWNER);
+      actionsWrapper =
+          coverageReportActionFactory.createCoverageReportActionsWrapper(
+              eventHandler,
+              directories,
+              allTargetsToTest,
+              baselineCoverageArtifacts,
+              getArtifactFactory(),
+              CoverageReportValue.COVERAGE_REPORT_KEY);
       if (actionsWrapper != null) {
         ImmutableList<ActionAnalysisMetadata> actions = actionsWrapper.getActions();
         skyframeExecutor.injectCoverageReportData(actions);
@@ -673,7 +676,7 @@ public class BuildView {
           public ActionAnalysisMetadata getGeneratingAction(Artifact artifact) {
             ArtifactOwner artifactOwner = artifact.getArtifactOwner();
             if (artifactOwner instanceof ActionLookupValue.ActionLookupKey) {
-              SkyKey key = ActionLookupValue.key((ActionLookupValue.ActionLookupKey) artifactOwner);
+              SkyKey key = (ActionLookupValue.ActionLookupKey) artifactOwner;
               ActionLookupValue val;
               try {
                 val = (ActionLookupValue) graph.getValue(key);
@@ -1081,7 +1084,7 @@ public class BuildView {
         new CachingAnalysisEnvironment(
             getArtifactFactory(),
             skyframeExecutor.getActionKeyContext(),
-            new ConfiguredTargetKey(target.getLabel(), targetConfig),
+            ConfiguredTargetKey.of(target.getLabel(), targetConfig),
             /*isSystemEnv=*/ false,
             targetConfig.extendedSanityChecks(),
             eventHandler,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index 7d17950e2c..81d5d2e23f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -297,14 +297,14 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment {
 
   @Override
   public Artifact getStableWorkspaceStatusArtifact() throws InterruptedException {
-    return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.SKY_KEY))
-            .getStableArtifact();
+    return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
+        .getStableArtifact();
   }
 
   @Override
   public Artifact getVolatileWorkspaceStatusArtifact() throws InterruptedException {
-    return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.SKY_KEY))
-            .getVolatileArtifact();
+    return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
+        .getVolatileArtifact();
   }
 
   // See SkyframeBuildView#getWorkspaceStatusValues for the code that this method is attempting to
@@ -327,8 +327,7 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment {
       throws InterruptedException {
     boolean stamp = AnalysisUtils.isStampingEnabled(ruleContext, config);
     BuildInfoCollectionValue collectionValue =
-        (BuildInfoCollectionValue) skyframeEnv.getValue(BuildInfoCollectionValue.key(
-            new BuildInfoCollectionValue.BuildInfoKeyAndConfig(key, config)));
+        (BuildInfoCollectionValue) skyframeEnv.getValue(BuildInfoCollectionValue.key(key, config));
     if (collectionValue == null) {
       throw collectDebugInfoAndCrash(key, config);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 25a33be726..913b6f1a71 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -166,8 +166,10 @@ public final class ConfiguredTargetFactory {
     Root root = rule.hasBinaryOutput()
         ? configuration.getBinDirectory(rule.getRepository())
         : configuration.getGenfilesDirectory(rule.getRepository());
-    ArtifactOwner owner = new ConfiguredTargetKey(rule.getLabel(),
-        getArtifactOwnerConfiguration(analysisEnvironment.getSkyframeEnv(), configuration));
+    ArtifactOwner owner =
+        ConfiguredTargetKey.of(
+            rule.getLabel(),
+            getArtifactOwnerConfiguration(analysisEnvironment.getSkyframeEnv(), configuration));
     if (analysisEnvironment.getSkyframeEnv().valuesMissing()) {
       return null;
     }
@@ -271,11 +273,13 @@ public final class ConfiguredTargetFactory {
       }
     } else if (target instanceof InputFile) {
       InputFile inputFile = (InputFile) target;
-      Artifact artifact = artifactFactory.getSourceArtifact(
-          inputFile.getExecPath(),
-          Root.asSourceRoot(inputFile.getPackage().getSourceRoot(),
-              inputFile.getPackage().getPackageIdentifier().getRepository().isMain()),
-          new ConfiguredTargetKey(target.getLabel(), config));
+      Artifact artifact =
+          artifactFactory.getSourceArtifact(
+              inputFile.getExecPath(),
+              Root.asSourceRoot(
+                  inputFile.getPackage().getSourceRoot(),
+                  inputFile.getPackage().getPackageIdentifier().getRepository().isMain()),
+              ConfiguredTargetKey.of(target.getLabel(), config));
 
       return new InputFileConfiguredTarget(targetContext, inputFile, artifact);
     } else if (target instanceof PackageGroup) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
index e64098d39e..21ed49ed98 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetAndConfiguration.java
@@ -13,13 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import java.util.Objects;
 import javax.annotation.Nullable;
 
@@ -49,22 +47,6 @@ public final class TargetAndConfiguration {
         + (configuration == null ? "null" : configuration.checksum());
   }
 
-  public static final Function<TargetAndConfiguration, String> NAME_FUNCTION =
-      new Function<TargetAndConfiguration, String>() {
-        @Override
-        public String apply(TargetAndConfiguration node) {
-          return node.getName();
-        }
-      };
-
-  public static final Function<TargetAndConfiguration, ConfiguredTargetKey>
-      TO_LABEL_AND_CONFIGURATION = new Function<TargetAndConfiguration, ConfiguredTargetKey>() {
-        @Override
-        public ConfiguredTargetKey apply(TargetAndConfiguration input) {
-          return new ConfiguredTargetKey(input.getLabel(), input.getConfiguration());
-        }
-      };
-
   @Override
   public boolean equals(Object that) {
     if (this == that) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 4f7bce06a0..549341a872 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -474,7 +474,7 @@ public class CppCompileAction extends AbstractAction
     }
     Map<Artifact, SkyKey> skyKeys = new HashMap<>();
     for (Artifact artifact : this.usedModules) {
-      skyKeys.put(artifact, ActionLookupValue.key((ActionLookupKey) artifact.getArtifactOwner()));
+      skyKeys.put(artifact, (ActionLookupKey) artifact.getArtifactOwner());
     }
     Map<SkyKey, SkyValue> skyValues = env.getValues(skyKeys.values());
     Set<Artifact> additionalModules = Sets.newLinkedHashSet();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
index 7771e08ce2..0897116f7b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -15,20 +15,19 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.analysis.actions.ActionTemplate;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.skyframe.LegacySkyKey;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
 
 /**
  * Value that stores expanded actions from ActionTemplate.
  */
 public final class ActionTemplateExpansionValue extends ActionLookupValue {
-
   ActionTemplateExpansionValue(
       ActionKeyContext actionKeyContext,
       Iterable<Action> expandedActions,
@@ -36,21 +35,17 @@ public final class ActionTemplateExpansionValue extends ActionLookupValue {
     super(actionKeyContext, ImmutableList.copyOf(expandedActions), removeActionsAfterEvaluation);
   }
 
-  static SkyKey key(ActionTemplate<?> actionTemplate) {
-    return LegacySkyKey.create(
-        SkyFunctions.ACTION_TEMPLATE_EXPANSION, createActionTemplateExpansionKey(actionTemplate));
-  }
+  private static final Interner<ActionTemplateExpansionKey> interner =
+      BlazeInterners.newWeakInterner();
 
-  static ActionTemplateExpansionKey createActionTemplateExpansionKey(
-      ActionTemplate<?> actionTemplate) {
-    return new ActionTemplateExpansionKey(actionTemplate);
+  static ActionTemplateExpansionKey key(ActionTemplate<?> actionTemplate) {
+    return interner.intern(new ActionTemplateExpansionKey(actionTemplate));
   }
 
-
   static final class ActionTemplateExpansionKey extends ActionLookupKey {
     private final ActionTemplate<?> actionTemplate;
 
-    ActionTemplateExpansionKey(ActionTemplate<?> actionTemplate) {
+    private ActionTemplateExpansionKey(ActionTemplate<?> actionTemplate) {
       Preconditions.checkNotNull(
           actionTemplate,
           "Passed in action template cannot be null: %s",
@@ -59,11 +54,10 @@ public final class ActionTemplateExpansionValue extends ActionLookupValue {
     }
 
     @Override
-    protected SkyFunctionName getType() {
+    public SkyFunctionName functionName() {
       return SkyFunctions.ACTION_TEMPLATE_EXPANSION;
     }
 
-
     @Override
     public Label getLabel() {
       return actionTemplate.getOwner().getLabel();
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 bb0ddbacd9..10016a1e5d 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
@@ -315,7 +315,7 @@ class ArtifactFunction implements SkyFunction {
     ArtifactOwner artifactOwner = artifact.getArtifactOwner();
 
     Preconditions.checkState(artifactOwner instanceof ActionLookupKey, "", artifact, artifactOwner);
-    return ActionLookupValue.key((ActionLookupKey) artifactOwner);
+    return (ActionLookupKey) artifactOwner;
   }
 
   @Nullable
@@ -326,7 +326,7 @@ class ArtifactFunction implements SkyFunction {
     if (value == null) {
       ArtifactOwner artifactOwner = artifact.getArtifactOwner();
       Preconditions.checkState(
-          artifactOwner == CoverageReportValue.ARTIFACT_OWNER,
+          artifactOwner == CoverageReportValue.COVERAGE_REPORT_KEY,
           "Not-yet-present artifact owner: %s (%s %s)",
           artifactOwner,
           artifact,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index f3d47e3763..56599c1ab8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.analysis.AliasProvider;
 import com.google.devtools.build.lib.analysis.AspectResolver;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -408,8 +407,7 @@ public final class AspectFunction implements SkyFunction {
       throws DuplicateException {
     ArrayList<ConfiguredAspect> aspectValues = new ArrayList<>();
     for (AspectKey aspectKey : aspectKeys) {
-      SkyKey skyAspectKey = aspectKey.getSkyKey();
-      AspectValue aspectValue = (AspectValue) values.get(skyAspectKey);
+      AspectValue aspectValue = (AspectValue) values.get(aspectKey);
       ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
       aspectValues.add(configuredAspect);
     }
@@ -438,14 +436,13 @@ public final class AspectFunction implements SkyFunction {
       return;
     }
     ImmutableList<AspectKey> baseKeys = key.getBaseKeys();
-    SkyKey skyKey = key.getSkyKey();
-    result.put(key.getAspectDescriptor(), skyKey);
+    result.put(key.getAspectDescriptor(), key);
     for (AspectKey baseKey : baseKeys) {
       buildSkyKeys(baseKey, result, aspectPathBuilder);
     }
     // Post-order list of aspect SkyKeys gives the order of propagating aspects:
     // the aspect comes after all aspects it transitively sees.
-    aspectPathBuilder.add(skyKey);
+    aspectPathBuilder.add(key);
   }
 
   private SkyValue createAliasAspect(
@@ -462,7 +459,7 @@ public final class AspectFunction implements SkyFunction {
     // the real configured target.
     Label aliasLabel = aliasChain.size() > 1 ? aliasChain.get(1) : configuredTarget.getLabel();
 
-    SkyKey depKey = ActionLookupValue.key(originalKey.withLabel(aliasLabel));
+    SkyKey depKey = originalKey.withLabel(aliasLabel);
 
     // Compute the AspectValue of the target the alias refers to (which can itself be either an
     // alias or a real target)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index a6ecc98a77..b6c6e0d366 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
@@ -24,6 +25,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Aspect;
 import com.google.devtools.build.lib.packages.AspectClass;
@@ -56,6 +58,7 @@ public final class AspectValue extends ActionLookupValue {
     private final BuildConfiguration aspectConfiguration;
     private final BuildConfiguration baseConfiguration;
     private final AspectDescriptor aspectDescriptor;
+    private int hashCode;
 
     private AspectKey(
         Label label,
@@ -71,7 +74,7 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     @Override
-    protected SkyFunctionName getType() {
+    public SkyFunctionName functionName() {
       return SkyFunctions.ASPECT;
     }
 
@@ -148,6 +151,31 @@ public final class AspectValue extends ActionLookupValue {
 
     @Override
     public int hashCode() {
+      // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+      // things going on here:
+      //
+      // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+      // Yes, this means that if the hash code is really 0 then we will "recompute" it each time.
+      // But this isn't a problem in practice since a hash code of 0 should be rare.
+      //
+      // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+      // first one to compute and cache the hash code.
+      //
+      // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+      // thread may not be visible by another.
+      //
+      // All three of these issues are benign from a correctness perspective; in the end we have no
+      // overhead from synchronization, at the cost of potentially computing the hash code more than
+      // once.
+      int h = hashCode;
+      if (h == 0) {
+        h = computeHashCode();
+        hashCode = h;
+      }
+      return h;
+    }
+
+    private int computeHashCode() {
       return Objects.hashCode(
           label,
           baseKeys,
@@ -227,6 +255,7 @@ public final class AspectValue extends ActionLookupValue {
     private final BuildConfiguration targetConfiguration;
     private final SkylarkImport skylarkImport;
     private final String skylarkValueName;
+    private int hashCode;
 
     private SkylarkAspectLoadingKey(
         Label targetLabel,
@@ -243,7 +272,7 @@ public final class AspectValue extends ActionLookupValue {
     }
 
     @Override
-    protected SkyFunctionName getType() {
+    public SkyFunctionName functionName() {
       return SkyFunctions.LOAD_SKYLARK_ASPECT;
     }
 
@@ -282,6 +311,31 @@ public final class AspectValue extends ActionLookupValue {
 
     @Override
     public int hashCode() {
+      // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+      // things going on here:
+      //
+      // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+      // Yes, this means that if the hash code is really 0 then we will "recompute" it each time.
+      // But this isn't a problem in practice since a hash code of 0 should be rare.
+      //
+      // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+      // first one to compute and cache the hash code.
+      //
+      // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+      // thread may not be visible by another.
+      //
+      // All three of these issues are benign from a correctness perspective; in the end we have no
+      // overhead from synchronization, at the cost of potentially computing the hash code more than
+      // once.
+      int h = hashCode;
+      if (h == 0) {
+        h = computeHashCode();
+        hashCode = h;
+      }
+      return h;
+    }
+
+    private int computeHashCode() {
       return Objects.hashCode(targetLabel,
           aspectConfiguration,
           targetConfiguration,
@@ -392,24 +446,27 @@ public final class AspectValue extends ActionLookupValue {
       ImmutableList<AspectKey> baseKeys,
       AspectDescriptor aspectDescriptor,
       BuildConfiguration aspectConfiguration) {
-    return new AspectKey(
+    return aspectKeyInterner.intern(new AspectKey(
         label, baseConfiguration,
         baseKeys,
         aspectDescriptor,
         aspectConfiguration
-    );
+    ));
   }
 
+  private static final Interner<AspectKey> aspectKeyInterner = BlazeInterners.newWeakInterner();
 
   public static AspectKey createAspectKey(
       Label label,
       BuildConfiguration baseConfiguration, AspectDescriptor aspectDescriptor,
       BuildConfiguration aspectConfiguration) {
-    return new AspectKey(
-        label, baseConfiguration, ImmutableList.<AspectKey>of(),
-        aspectDescriptor, aspectConfiguration);
+    return aspectKeyInterner.intern(
+        new AspectKey(
+            label, baseConfiguration, ImmutableList.of(), aspectDescriptor, aspectConfiguration));
   }
 
+  private static final Interner<SkylarkAspectLoadingKey> skylarkAspectKeyInterner =
+      BlazeInterners.newWeakInterner();
 
   public static SkylarkAspectLoadingKey createSkylarkAspectKey(
       Label targetLabel,
@@ -417,7 +474,12 @@ public final class AspectValue extends ActionLookupValue {
       BuildConfiguration targetConfiguration,
       SkylarkImport skylarkImport,
       String skylarkExportName) {
-    return new SkylarkAspectLoadingKey(
-        targetLabel, aspectConfiguration, targetConfiguration, skylarkImport, skylarkExportName);
+    return skylarkAspectKeyInterner.intern(
+        new SkylarkAspectLoadingKey(
+            targetLabel,
+            aspectConfiguration,
+            targetConfiguration,
+            skylarkImport,
+            skylarkExportName));
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
index 62878a5fe2..c80875d7d8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -58,7 +58,7 @@ public class BuildInfoCollectionFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
     final BuildInfoKeyAndConfig keyAndConfig = (BuildInfoKeyAndConfig) skyKey.argument();
     WorkspaceStatusValue infoArtifactValue =
-        (WorkspaceStatusValue) env.getValue(WorkspaceStatusValue.SKY_KEY);
+        (WorkspaceStatusValue) env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
     if (infoArtifactValue == null) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
index 64869399b5..8695042cca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
@@ -14,12 +14,14 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import java.util.Objects;
 
@@ -47,18 +49,26 @@ public class BuildInfoCollectionValue extends ActionLookupValue {
     return getStringHelper().add("collection", collection).toString();
   }
 
+  private static final Interner<BuildInfoKeyAndConfig> keyInterner =
+      BlazeInterners.newWeakInterner();
+
+  public static BuildInfoKeyAndConfig key(
+      BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
+    return keyInterner.intern(new BuildInfoKeyAndConfig(key, config));
+  }
+
   /** Key for BuildInfoCollectionValues. */
   public static class BuildInfoKeyAndConfig extends ActionLookupKey {
     private final BuildInfoFactory.BuildInfoKey infoKey;
     private final BuildConfiguration config;
 
-    public BuildInfoKeyAndConfig(BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
+    private BuildInfoKeyAndConfig(BuildInfoFactory.BuildInfoKey key, BuildConfiguration config) {
       this.infoKey = Preconditions.checkNotNull(key, config);
       this.config = Preconditions.checkNotNull(config, key);
     }
 
     @Override
-    protected SkyFunctionName getType() {
+    public SkyFunctionName functionName() {
       return SkyFunctions.BUILD_INFO_COLLECTION;
     }
 
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 02bb53fd10..db8d44ccc2 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
@@ -103,8 +103,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
     public ConfiguredTargetValue getValueFromSkyKey(SkyKey skyKey, Environment env)
         throws InterruptedException {
       TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
-      ConfiguredTargetKey lac = tcKey.configuredTargetKey();
-      return (ConfiguredTargetValue) env.getValue(lac.getSkyKey());
+      return (ConfiguredTargetValue) env.getValue(tcKey.configuredTargetKey());
     }
 
     @Override
@@ -179,7 +178,7 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S
         throws InterruptedException {
       AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
       AspectKey aspectKey = acKey.aspectKey();
-      return (AspectValue) env.getValue(aspectKey.getSkyKey());
+      return (AspectValue) env.getValue(aspectKey);
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index af83e3c9a6..258223b3f0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -604,9 +604,13 @@ public final class ConfiguredTargetFunction implements SkyFunction {
     if (env.valuesMissing()) {
       return null;
     }
-    CachingAnalysisEnvironment analysisEnvironment = view.createAnalysisEnvironment(
-        new ConfiguredTargetKey(target.getLabel(), ownerConfig), false,
-        events, env, configuration);
+    CachingAnalysisEnvironment analysisEnvironment =
+        view.createAnalysisEnvironment(
+            ConfiguredTargetKey.of(target.getLabel(), ownerConfig),
+            false,
+            events,
+            env,
+            configuration);
     if (env.valuesMissing()) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
index c02fd999d9..6cdd9599c6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -14,13 +14,14 @@
 
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
 import com.google.devtools.build.lib.analysis.AliasProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import java.util.Objects;
 import javax.annotation.Nullable;
@@ -36,16 +37,13 @@ public class ConfiguredTargetKey extends ActionLookupKey {
   @Nullable
   private final BuildConfiguration configuration;
 
-  public ConfiguredTargetKey(Label label, @Nullable BuildConfiguration configuration) {
+  private transient int hashCode;
+
+  private ConfiguredTargetKey(Label label, @Nullable BuildConfiguration configuration) {
     this.label = Preconditions.checkNotNull(label);
     this.configuration = configuration;
   }
 
-  @VisibleForTesting
-  public ConfiguredTargetKey(ConfiguredTarget rule) {
-    this(rule.getTarget().getLabel(), rule.getConfiguration());
-  }
-
   public static ConfiguredTargetKey of(ConfiguredTarget configuredTarget) {
     AliasProvider aliasProvider = configuredTarget.getProvider(AliasProvider.class);
     Label label =
@@ -53,8 +51,14 @@ public class ConfiguredTargetKey extends ActionLookupKey {
     return of(label, configuredTarget.getConfiguration());
   }
 
+  /**
+   * Cache so that the number of ConfiguredTargetKey instances is {@code O(configured targets)} and
+   * not {@code O(edges between configured targets)}.
+   */
+  private static final Interner<ConfiguredTargetKey> interner = BlazeInterners.newWeakInterner();
+
   public static ConfiguredTargetKey of(Label label, @Nullable BuildConfiguration configuration) {
-    return new ConfiguredTargetKey(label, configuration);
+    return interner.intern(new ConfiguredTargetKey(label, configuration));
   }
 
   @Override
@@ -63,7 +67,7 @@ public class ConfiguredTargetKey extends ActionLookupKey {
   }
 
   @Override
-  protected SkyFunctionName getType() {
+  public SkyFunctionName functionName() {
     return SkyFunctions.CONFIGURED_TARGET;
   }
 
@@ -74,6 +78,31 @@ public class ConfiguredTargetKey extends ActionLookupKey {
 
   @Override
   public int hashCode() {
+    // We use the hash code caching strategy employed by java.lang.String. There are three subtle
+    // things going on here:
+    //
+    // (1) We use a value of 0 to indicate that the hash code hasn't been computed and cached yet.
+    // Yes, this means that if the hash code is really 0 then we will "recompute" it each time. But
+    // this isn't a problem in practice since a hash code of 0 should be rare.
+    //
+    // (2) Since we have no synchronization, multiple threads can race here thinking there are the
+    // first one to compute and cache the hash code.
+    //
+    // (3) Moreover, since 'hashCode' is non-volatile, the cached hash code value written from one
+    // thread may not be visible by another.
+    //
+    // All three of these issues are benign from a correctness perspective; in the end we have no
+    // overhead from synchronization, at the cost of potentially computing the hash code more than
+    // once.
+    int h = hashCode;
+    if (h == 0) {
+      h = computeHashCode();
+      hashCode = h;
+    }
+    return h;
+  }
+
+  private int computeHashCode() {
     int configVal = configuration == null ? 79 : configuration.hashCode();
     return 31 * label.hashCode() + configVal;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
index a11e27a092..fc4ebd6727 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
@@ -100,13 +100,13 @@ public final class ConfiguredTargetValue extends ActionLookupValue {
 
   @VisibleForTesting
   public static SkyKey key(Label label, BuildConfiguration configuration) {
-    return key(new ConfiguredTargetKey(label, configuration));
+    return ConfiguredTargetKey.of(label, configuration);
   }
 
   static ImmutableList<SkyKey> keys(Iterable<ConfiguredTargetKey> lacs) {
     ImmutableList.Builder<SkyKey> keys = ImmutableList.builder();
     for (ConfiguredTargetKey lac : lacs) {
-      keys.add(key(lac));
+      keys.add(lac);
     }
     return keys.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
index 64fdcc4fb5..3fbf2d9e2d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
@@ -41,8 +41,10 @@ public class CoverageReportFunction implements SkyFunction {
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
     Preconditions.checkState(
-        CoverageReportValue.SKY_KEY.equals(skyKey), String.format(
-            "Expected %s for SkyKey but got %s instead", CoverageReportValue.SKY_KEY, skyKey));
+        CoverageReportValue.COVERAGE_REPORT_KEY.equals(skyKey),
+        String.format(
+            "Expected %s for SkyKey but got %s instead",
+            CoverageReportValue.COVERAGE_REPORT_KEY, skyKey));
 
     ImmutableList<ActionAnalysisMetadata> actions = PrecomputedValue.COVERAGE_REPORT_KEY.get(env);
     if (actions == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
index fde0599749..fd9bace71e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
@@ -18,10 +18,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
-import com.google.devtools.build.skyframe.LegacySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
 
 /**
  * A SkyValue to store the coverage report Action and Artifacts.
@@ -29,8 +26,7 @@ import com.google.devtools.build.skyframe.SkyKey;
 public class CoverageReportValue extends ActionLookupValue {
 
   // There should only ever be one CoverageReportValue value in the graph.
-  public static final ArtifactOwner ARTIFACT_OWNER = new CoverageReportKey();
-  static final SkyKey SKY_KEY = LegacySkyKey.create(SkyFunctions.COVERAGE_REPORT, ARTIFACT_OWNER);
+  public static final CoverageReportKey COVERAGE_REPORT_KEY = new CoverageReportKey();
 
   CoverageReportValue(
       ActionKeyContext actionKeyContext,
@@ -39,15 +35,13 @@ public class CoverageReportValue extends ActionLookupValue {
     super(actionKeyContext, coverageReportActions, removeActionsAfterEvaluation);
   }
 
-  private static class CoverageReportKey extends ActionLookupKey {
-    @Override
-    protected SkyFunctionName getType() {
-      throw new UnsupportedOperationException();
-    }
+  static class CoverageReportKey extends ActionLookupKey {
+    private CoverageReportKey() {}
 
     @Override
-    protected SkyKey getSkyKeyInternal() {
-      return SKY_KEY;
+    public SkyFunctionName functionName() {
+      return SkyFunctions.COVERAGE_REPORT;
     }
+
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
index 779dbaa255..3ae57a524e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
@@ -51,12 +51,12 @@ import javax.annotation.Nullable;
 public class PostConfiguredTargetFunction implements SkyFunction {
   private static final Function<Dependency, SkyKey> TO_KEYS =
       new Function<Dependency, SkyKey>() {
-    @Override
-    public SkyKey apply(Dependency input) {
-      return PostConfiguredTargetValue.key(
-          new ConfiguredTargetKey(input.getLabel(), input.getConfiguration()));
-    }
-  };
+        @Override
+        public SkyKey apply(Dependency input) {
+          return PostConfiguredTargetValue.key(
+              ConfiguredTargetKey.of(input.getLabel(), input.getConfiguration()));
+        }
+      };
 
   private final SkyframeExecutor.BuildViewProvider buildViewProvider;
   private final RuleClassProvider ruleClassProvider;
@@ -74,8 +74,8 @@ public class PostConfiguredTargetFunction implements SkyFunction {
       throws SkyFunctionException, InterruptedException {
     ImmutableMap<ActionAnalysisMetadata, ConflictException> badActions =
         PrecomputedValue.BAD_ACTIONS.get(env);
-    ConfiguredTargetValue ctValue = (ConfiguredTargetValue)
-        env.getValue(ConfiguredTargetValue.key((ConfiguredTargetKey) skyKey.argument()));
+    ConfiguredTargetValue ctValue =
+        (ConfiguredTargetValue) env.getValue((ConfiguredTargetKey) skyKey.argument());
     if (env.valuesMissing()) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
index d8f7b1b46c..ad531b30f5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
@@ -106,7 +106,7 @@ public class RegisteredToolchainsFunction implements SkyFunction {
                 label ->
                     LegacySkyKey.create(
                         SkyFunctions.CONFIGURED_TARGET,
-                        new ConfiguredTargetKey(label, configuration)))
+                        ConfiguredTargetKey.of(label, configuration)))
             .collect(ImmutableList.toImmutableList());
 
     Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> values =
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 8adef6d163..0313d477d8 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
@@ -743,7 +743,6 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         // well as ActionExecutionValues, since they do not depend on ActionLookupValues.
         SkyFunctionName.functionIsIn(ImmutableSet.of(
             SkyFunctions.CONFIGURED_TARGET,
-            SkyFunctions.ACTION_LOOKUP,
             SkyFunctions.BUILD_INFO,
             SkyFunctions.TARGET_COMPLETION,
             SkyFunctions.BUILD_INFO_COLLECTION,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index 4f7479b5d6..c701fa6177 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -86,7 +86,6 @@ public final class SkyFunctions {
   public static final SkyFunctionName ARTIFACT = SkyFunctionName.create("ARTIFACT");
   public static final SkyFunctionName ACTION_EXECUTION =
       SkyFunctionName.create("ACTION_EXECUTION");
-  public static final SkyFunctionName ACTION_LOOKUP = SkyFunctionName.create("ACTION_LOOKUP");
   public static final SkyFunctionName RECURSIVE_FILESYSTEM_TRAVERSAL =
       SkyFunctionName.create("RECURSIVE_DIRECTORY_TRAVERSAL");
   public static final SkyFunctionName FILESET_ENTRY = SkyFunctionName.create("FILESET_ENTRY");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index a62d501ccb..a137435872 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -60,7 +60,6 @@ import com.google.devtools.build.lib.pkgcache.LoadingFailureEvent;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
-import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
 import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
@@ -221,7 +220,7 @@ public final class SkyframeBuildView {
     NestedSetBuilder<Package> packages =
         singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null;
     for (AspectValueKey aspectKey : aspectKeys) {
-      AspectValue value = (AspectValue) result.get(aspectKey.getSkyKey());
+      AspectValue value = (AspectValue) result.get(aspectKey);
       if (value == null) {
         // Skip aspects that couldn't be applied to targets.
         continue;
@@ -237,8 +236,7 @@ public final class SkyframeBuildView {
     // list of values, i.e., that the order is deterministic.
     Collection<ConfiguredTarget> goodCts = Lists.newArrayListWithCapacity(values.size());
     for (ConfiguredTargetKey value : values) {
-      ConfiguredTargetValue ctValue =
-          (ConfiguredTargetValue) result.get(ConfiguredTargetValue.key(value));
+      ConfiguredTargetValue ctValue = (ConfiguredTargetValue) result.get(value);
       if (ctValue == null) {
         continue;
       }
@@ -450,13 +448,13 @@ public final class SkyframeBuildView {
       BuildConfiguration config,
       ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories)
       throws InterruptedException {
-    env.getValue(WorkspaceStatusValue.SKY_KEY);
+    env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
     // These factories may each create their own build info artifacts, all depending on the basic
     // build-info.txt and build-changelist.txt.
     List<SkyKey> depKeys = Lists.newArrayList();
     for (BuildInfoKey key : buildInfoFactories.keySet()) {
       if (buildInfoFactories.get(key).isEnabled(config)) {
-        depKeys.add(BuildInfoCollectionValue.key(new BuildInfoKeyAndConfig(key, config)));
+        depKeys.add(BuildInfoCollectionValue.key(key, config));
       }
     }
     env.getValues(depKeys);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index a59f8652af..429caf8344 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -752,20 +752,21 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       // TODO(janakr): don't invalidate here, just use different keys for different configs. Can't
       // be done right now because of lack of configuration trimming and fact that everything
       // depends on workspace status action.
-      invalidate(WorkspaceStatusValue.SKY_KEY::equals);
+      invalidate(WorkspaceStatusValue.BUILD_INFO_KEY::equals);
     }
   }
 
   private WorkspaceStatusAction makeWorkspaceStatusAction(String workspaceName) {
     return workspaceStatusActionFactory.createWorkspaceStatusAction(
-        artifactFactory.get(), WorkspaceStatusValue.ARTIFACT_OWNER, buildId, workspaceName);
+        artifactFactory.get(), WorkspaceStatusValue.BUILD_INFO_KEY, buildId, workspaceName);
   }
 
   @VisibleForTesting
   @Nullable
   public WorkspaceStatusAction getLastWorkspaceStatusAction() throws InterruptedException {
     WorkspaceStatusValue workspaceStatusValue =
-        (WorkspaceStatusValue) memoizingEvaluator.getExistingValue(WorkspaceStatusValue.SKY_KEY);
+        (WorkspaceStatusValue)
+            memoizingEvaluator.getExistingValue(WorkspaceStatusValue.BUILD_INFO_KEY);
     return workspaceStatusValue == null
         ? null
         : (WorkspaceStatusAction) workspaceStatusValue.getAction(0);
@@ -818,11 +819,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   public Collection<Artifact> getWorkspaceStatusArtifacts(ExtendedEventHandler eventHandler)
       throws InterruptedException {
     // Should already be present, unless the user didn't request any targets for analysis.
-    EvaluationResult<WorkspaceStatusValue> result = buildDriver.evaluate(
-        ImmutableList.of(WorkspaceStatusValue.SKY_KEY), /*keepGoing=*/true, /*numThreads=*/1,
-        eventHandler);
+    EvaluationResult<WorkspaceStatusValue> result =
+        buildDriver.evaluate(
+            ImmutableList.of(WorkspaceStatusValue.BUILD_INFO_KEY),
+            /*keepGoing=*/ true,
+            /*numThreads=*/ 1,
+            eventHandler);
     WorkspaceStatusValue value =
-        Preconditions.checkNotNull(result.get(WorkspaceStatusValue.SKY_KEY));
+        Preconditions.checkNotNull(result.get(WorkspaceStatusValue.BUILD_INFO_KEY));
     return ImmutableList.of(value.getStableArtifact(), value.getVolatileArtifact());
   }
 
@@ -1313,8 +1317,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       for (BuildConfiguration depConfig : configs.get(key)) {
         skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), depConfig));
         for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
-          skyKeys.add(ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(), depConfig,
-              aspectDescriptor, depConfig)));
+          skyKeys.add(
+              AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig));
         }
       }
     }
@@ -1346,8 +1350,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         List<ConfiguredAspect> configuredAspects = new ArrayList<>();
 
         for (AspectDescriptor aspectDescriptor : key.getAspects().getAllAspects()) {
-          SkyKey aspectKey = ActionLookupValue.key(AspectValue.createAspectKey(key.getLabel(),
-              depConfig, aspectDescriptor, depConfig));
+          SkyKey aspectKey =
+              AspectValue.createAspectKey(key.getLabel(), depConfig, aspectDescriptor, depConfig);
           if (result.get(aspectKey) == null) {
             continue DependentNodeLoop;
           }
@@ -1642,7 +1646,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
     List<SkyKey> keys = new ArrayList<>(ConfiguredTargetValue.keys(values));
     for (AspectValueKey aspectKey : aspectKeys) {
-      keys.add(aspectKey.getSkyKey());
+      keys.add(aspectKey);
     }
     EvaluationResult<ActionLookupValue> result =
         buildDriver.evaluate(keys, keepGoing, numThreads, eventHandler);
@@ -1746,8 +1750,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     ArtifactOwner artifactOwner = artifact.getArtifactOwner();
     Preconditions.checkState(artifactOwner instanceof ActionLookupValue.ActionLookupKey,
         "%s %s", artifact, artifactOwner);
-    SkyKey actionLookupKey =
-        ActionLookupValue.key((ActionLookupValue.ActionLookupKey) artifactOwner);
+    SkyKey actionLookupKey = (ActionLookupValue.ActionLookupKey) artifactOwner;
 
     synchronized (valueLookupLock) {
       // Note that this will crash (attempting to run a configured target value builder after
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
index 32e5ec5f0b..4a9946a1bc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
@@ -32,13 +32,13 @@ public final class TestCompletionFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
     TestCompletionValue.TestCompletionKey key =
         (TestCompletionValue.TestCompletionKey) skyKey.argument();
-    ConfiguredTargetKey lac = key.configuredTargetKey();
+    ConfiguredTargetKey ctKey = key.configuredTargetKey();
     TopLevelArtifactContext ctx = key.topLevelArtifactContext();
-    if (env.getValue(TargetCompletionValue.key(lac, ctx, /*willTest=*/true)) == null) {
+    if (env.getValue(TargetCompletionValue.key(ctKey, ctx, /*willTest=*/ true)) == null) {
       return null;
     }
 
-    ConfiguredTargetValue ctValue = (ConfiguredTargetValue) env.getValue(lac.getSkyKey());
+    ConfiguredTargetValue ctValue = (ConfiguredTargetValue) env.getValue(ctKey);
     if (ctValue == null) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
index a47e373906..11e44fca77 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
@@ -144,11 +144,11 @@ public class ToolchainUtil {
     SkyKey executionPlatformKey =
         LegacySkyKey.create(
             SkyFunctions.CONFIGURED_TARGET,
-            new ConfiguredTargetKey(executionPlatformLabel, configuration));
+            ConfiguredTargetKey.of(executionPlatformLabel, configuration));
     SkyKey targetPlatformKey =
         LegacySkyKey.create(
             SkyFunctions.CONFIGURED_TARGET,
-            new ConfiguredTargetKey(targetPlatformLabel, configuration));
+            ConfiguredTargetKey.of(targetPlatformLabel, configuration));
 
     Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> values =
         env.getValuesOrThrow(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
index e9ee29ed7c..82de66d23c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
@@ -84,11 +83,11 @@ public class ToplevelSkylarkAspectFunction implements SkyFunction {
       throw new LoadSkylarkAspectFunctionException(e);
     }
     SkyKey aspectKey =
-        ActionLookupValue.key(AspectValue.createAspectKey(
-            aspectLoadingKey.getTargetLabel(), aspectLoadingKey.getTargetConfiguration(),
+        AspectValue.createAspectKey(
+            aspectLoadingKey.getTargetLabel(),
+            aspectLoadingKey.getTargetConfiguration(),
             new AspectDescriptor(skylarkAspect.getAspectClass(), AspectParameters.EMPTY),
-            aspectLoadingKey.getAspectConfiguration()
-        ));
+            aspectLoadingKey.getAspectConfiguration());
 
     return env.getValue(aspectKey);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
index 817c2fab54..b231bae76a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusFunction.java
@@ -43,7 +43,7 @@ public class WorkspaceStatusFunction implements SkyFunction {
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
     Preconditions.checkState(
-        WorkspaceStatusValue.SKY_KEY.equals(skyKey), WorkspaceStatusValue.SKY_KEY);
+        WorkspaceStatusValue.BUILD_INFO_KEY.equals(skyKey), WorkspaceStatusValue.BUILD_INFO_KEY);
     WorkspaceNameValue workspaceNameValue =
         (WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
     if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
index 78ff03d0f3..1b695928a4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -16,11 +16,8 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionLookupValue;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
-import com.google.devtools.build.skyframe.LegacySkyKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
 
 /**
  * Value that stores the workspace status artifacts and their generating action. There should be
@@ -33,8 +30,7 @@ public class WorkspaceStatusValue extends ActionLookupValue {
   private final Artifact volatileArtifact;
 
   // There should only ever be one BuildInfo value in the graph.
-  static final ArtifactOwner ARTIFACT_OWNER = new BuildInfoKey();
-  public static final SkyKey SKY_KEY = LegacySkyKey.create(SkyFunctions.BUILD_INFO, ARTIFACT_OWNER);
+  public static final BuildInfoKey BUILD_INFO_KEY = new BuildInfoKey();
 
   WorkspaceStatusValue(
       ActionKeyContext actionKeyContext,
@@ -55,15 +51,13 @@ public class WorkspaceStatusValue extends ActionLookupValue {
     return volatileArtifact;
   }
 
-  private static class BuildInfoKey extends ActionLookupKey {
-    @Override
-    protected SkyFunctionName getType() {
-      throw new UnsupportedOperationException();
-    }
+  static class BuildInfoKey extends ActionLookupKey {
+    private BuildInfoKey() {}
 
     @Override
-    protected SkyKey getSkyKeyInternal() {
-      return SKY_KEY;
+    public SkyFunctionName functionName() {
+      return SkyFunctions.BUILD_INFO;
     }
+
   }
 }