Try to clean up some of the excess baggage Aspects are carting around, mainly from a code cleanliness perspective, but with some minor memory benefits:

1. Use AspectKey instead of AspectValue wherever possible at the top-level, since keys are what callers should be using, not values.
2. Remove the freestanding label field from AspectKey. It is always equal to its ConfiguredTargetKey field's label. Having two fields with redundant data is confusing.
3. Remove the freestanding label field from AspectValue. Same reason: it was always the AspectKey's label.
4. Remove AspectDescriptor from ConfiguredAspect: it comes from the AspectKey, so just use the key.

This is also good because I think maximizing correspondence between ConfiguredAspect and ConfiguredTarget is a goal. Having an AspectValue muddies the waters. Note that, for historical reasons, ConfiguredTarget has much more data in it than we'd want a ConfiguredAspect to have (ideally, both would just have actions and providers).

In a follow-up, I may wrap AspectValue together with an AspectKey in TopLevelSkylarkAspectFunction#compute's return value. I think that, plus some tolerable casting, would remove the only remaining real need for AspectValue to have an AspectKey inside it.

PiperOrigin-RevId: 307877866
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
index 901c8fc..dafb679 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -217,8 +217,7 @@
     update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.o");
     // We want to force a "dropConfiguredTargetsNow" operation, which won't inform the
     // invalidation receiver about the dropped configured targets.
-    skyframeExecutor.clearAnalysisCache(
-        ImmutableList.<ConfiguredTarget>of(), ImmutableSet.<AspectValue>of());
+    skyframeExecutor.clearAnalysisCache(ImmutableList.of(), ImmutableSet.of());
     assertContainsEvent("file 'conflict/_objs/x/foo.o' " + CONFLICT_MSG);
     eventCollector.clear();
     scratch.overwriteFile(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 51e5225..60164a6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -432,7 +432,7 @@
           String toolsRepository)
           throws InterruptedException, ActionConflictException {
         Object lateBoundPrereq = ruleContext.getPrerequisite(":late", TARGET);
-        return new ConfiguredAspect.Builder(this, parameters, ruleContext)
+        return new ConfiguredAspect.Builder(ruleContext)
             .addProvider(
                 AspectInfo.class,
                 new AspectInfo(
@@ -505,7 +505,7 @@
           String toolsRepository)
           throws InterruptedException, ActionConflictException {
         ruleContext.registerAction(new NullAction(ruleContext.createOutputArtifact()));
-        return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
+        return new ConfiguredAspect.Builder(ruleContext).build();
       }
     }
   }
@@ -832,9 +832,9 @@
     AnalysisResult analysisResult = update(new EventBus(), defaultFlags(),
         ImmutableList.of(aspectApplyingToFiles.getName()),
         "//a:x_deploy.jar");
-    AspectValue aspect = Iterables.getOnlyElement(analysisResult.getAspects());
+    ConfiguredAspect aspect = Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     AspectApplyingToFiles.Provider provider =
-        aspect.getConfiguredAspect().getProvider(AspectApplyingToFiles.Provider.class);
+        aspect.getProvider(AspectApplyingToFiles.Provider.class);
     assertThat(provider.getLabel())
         .isEqualTo(Label.parseAbsoluteUnchecked("//a:x_deploy.jar"));
   }
@@ -853,9 +853,8 @@
     AnalysisResult analysisResult = update(new EventBus(), defaultFlags(),
         ImmutableList.of(aspectApplyingToFiles.getName()),
         "//a:x.java");
-    AspectValue aspect = Iterables.getOnlyElement(analysisResult.getAspects());
-    assertThat(aspect.getConfiguredAspect().getProvider(AspectApplyingToFiles.Provider.class))
-        .isNull();
+    ConfiguredAspect aspect = Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
+    assertThat(aspect.getProvider(AspectApplyingToFiles.Provider.class)).isNull();
   }
 
   @Test
@@ -871,9 +870,9 @@
             defaultFlags(),
             ImmutableList.of(aspectApplyingToFiles.getName(), aspectApplyingToFiles.getName()),
             "//a:x_deploy.jar");
-    AspectValue aspect = Iterables.getOnlyElement(analysisResult.getAspects());
+    ConfiguredAspect aspect = Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     AspectApplyingToFiles.Provider provider =
-        aspect.getConfiguredAspect().getProvider(AspectApplyingToFiles.Provider.class);
+        aspect.getProvider(AspectApplyingToFiles.Provider.class);
     assertThat(provider.getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//a:x_deploy.jar"));
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 274c558..209ad60 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -404,7 +404,8 @@
             reporter,
             eventBus);
     if (discardAnalysisCache) {
-      buildView.clearAnalysisCache(analysisResult.getTargetsToBuild(), analysisResult.getAspects());
+      buildView.clearAnalysisCache(
+          analysisResult.getTargetsToBuild(), analysisResult.getAspectsMap().keySet());
     }
     masterConfig = analysisResult.getConfigurationCollection();
     return analysisResult;
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index cdc67d4..972d757 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -28,7 +28,6 @@
 import com.google.devtools.build.lib.analysis.AnalysisOptions;
 import com.google.devtools.build.lib.analysis.AnalysisResult;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
-import com.google.devtools.build.lib.analysis.AspectValue;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BuildView;
 import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
@@ -77,6 +76,7 @@
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
@@ -613,8 +613,9 @@
   }
 
   /** Clears the analysis cache as in --discard_analysis_cache. */
-  public void clearAnalysisCache(
-      Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+  @VisibleForTesting
+  void clearAnalysisCache(
+      Collection<ConfiguredTarget> topLevelTargets, ImmutableSet<AspectKey> topLevelAspects) {
     skyframeBuildView.clearAnalysisCache(topLevelTargets, topLevelAspects);
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index e4f25da..3177a5e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -239,7 +239,7 @@
       String information = parameters.isEmpty()
           ? ""
           : " data " + Iterables.getFirst(parameters.getAttribute("baz"), null);
-      return new ConfiguredAspect.Builder(this, parameters, ruleContext)
+      return new ConfiguredAspect.Builder(ruleContext)
           .addProvider(
               new AspectInfo(
                   collectAspectData("aspect " + ruleContext.getLabel() + information, ruleContext)))
@@ -286,9 +286,7 @@
         AspectParameters parameters,
         String toolsRepository)
         throws ActionConflictException {
-      return new ConfiguredAspect.Builder(this, parameters, ruleContext)
-          .addProvider(new FooProvider())
-          .build();
+      return new ConfiguredAspect.Builder(ruleContext).addProvider(new FooProvider()).build();
     }
   }
 
@@ -309,9 +307,7 @@
         AspectParameters parameters,
         String toolsRepository)
         throws ActionConflictException {
-      return new ConfiguredAspect.Builder(this, parameters, ruleContext)
-          .addProvider(new BarProvider())
-          .build();
+      return new ConfiguredAspect.Builder(ruleContext).addProvider(new BarProvider()).build();
     }
   }
 
@@ -463,7 +459,7 @@
         information.append(dep.getLabel());
       }
       information.append("]");
-      return new ConfiguredAspect.Builder(this, parameters, ruleContext)
+      return new ConfiguredAspect.Builder(ruleContext)
           .addProvider(new AspectInfo(collectAspectData(information.toString(), ruleContext)))
           .build();
     }
@@ -502,7 +498,7 @@
         String toolsRepository)
         throws ActionConflictException {
       ruleContext.ruleWarning("Aspect warning on " + ctadBase.getTarget().getLabel());
-      return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
+      return new ConfiguredAspect.Builder(ruleContext).build();
     }
 
     @Override
@@ -563,7 +559,7 @@
         AspectParameters parameters,
         String toolsRepository)
         throws InterruptedException, ActionConflictException {
-      return new ConfiguredAspect.Builder(this, parameters, context).build();
+      return new ConfiguredAspect.Builder(context).build();
     }
   }
   public static final FalseAdvertisementAspect FALSE_ADVERTISEMENT_ASPECT
@@ -812,7 +808,7 @@
         AspectParameters parameters,
         String toolsRepository)
         throws InterruptedException, ActionConflictException {
-      return ConfiguredAspect.builder(this, parameters, context)
+      return ConfiguredAspect.builder(context)
           .addProvider(Provider.class, new Provider(ctadBase.getConfiguredTarget().getLabel()))
           .build();
     }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/apple/AppleRulesTest.java b/src/test/java/com/google/devtools/build/lib/rules/apple/AppleRulesTest.java
index 64ece7d..e77bf68 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/apple/AppleRulesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/apple/AppleRulesTest.java
@@ -111,7 +111,7 @@
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
 
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java
index 8cc0d27..41bebf2 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java
@@ -59,7 +59,6 @@
 
     SkyKey aspectKey =
         AspectValueKey.AspectKey.createAspectKey(
-            makeLabel("//foo:a"),
             ctKey,
             ImmutableList.of(),
             null,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 43b0e9b..1262cfe 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -72,7 +72,6 @@
 import com.google.devtools.build.lib.analysis.AnalysisOptions;
 import com.google.devtools.build.lib.analysis.AnalysisProtos;
 import com.google.devtools.build.lib.analysis.AnalysisProtos.ActionGraphContainer;
-import com.google.devtools.build.lib.analysis.AspectValue;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.OutputGroupInfo;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
@@ -237,8 +236,7 @@
     ct = null;
     // Allow all values to be cleared by passing in empty set of top-level values, since we're not
     // actually building.
-    skyframeExecutor.clearAnalysisCache(
-        ImmutableSet.<ConfiguredTarget>of(), ImmutableSet.<AspectValue>of());
+    skyframeExecutor.clearAnalysisCache(ImmutableSet.of(), ImmutableSet.of());
     GcFinalization.awaitClear(ref);
   }
 
@@ -667,11 +665,11 @@
         reporter,
         ResourceManager.instanceForTestingOnly(),
         new DummyExecutor(fileSystem, rootDirectory),
-        ImmutableSet.<Artifact>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<AspectValue>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -763,11 +761,11 @@
         reporter,
         ResourceManager.instanceForTestingOnly(),
         new DummyExecutor(fileSystem, rootDirectory),
-        ImmutableSet.<Artifact>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<AspectValue>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -989,11 +987,11 @@
         reporter,
         ResourceManager.instanceForTestingOnly(),
         new DummyExecutor(fileSystem, rootDirectory),
-        ImmutableSet.<Artifact>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<AspectValue>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -1093,11 +1091,11 @@
         reporter,
         ResourceManager.instanceForTestingOnly(),
         new DummyExecutor(fileSystem, rootDirectory),
-        ImmutableSet.<Artifact>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<AspectValue>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -1312,11 +1310,11 @@
         reporter,
         ResourceManager.instanceForTestingOnly(),
         new DummyExecutor(fileSystem, rootDirectory),
-        ImmutableSet.<Artifact>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<AspectValue>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -1392,7 +1390,7 @@
         ImmutableSet.of(),
         ImmutableSet.of(),
         ImmutableSet.of(),
-        ImmutableSet.<ConfiguredTarget>of(),
+        ImmutableSet.of(),
         options,
         NULL_CHECKER,
         null,
@@ -1561,11 +1559,11 @@
                 builder.buildArtifacts(
                     reporter,
                     normalArtifacts,
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<AspectValue>of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
                     new DummyExecutor(fileSystem, rootDirectory),
                     builtTargets,
                     builtAspects,
@@ -1692,11 +1690,11 @@
                 builder.buildArtifacts(
                     reporter,
                     normalArtifacts,
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<AspectValue>of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
                     new DummyExecutor(fileSystem, rootDirectory),
                     builtTargets,
                     builtAspects,
@@ -1931,11 +1929,11 @@
                 builder.buildArtifacts(
                     reporter,
                     normalArtifacts,
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<AspectValue>of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
                     new DummyExecutor(fileSystem, rootDirectory),
                     builtTargets,
                     builtAspects,
@@ -2043,11 +2041,11 @@
                 builder.buildArtifacts(
                     reporter,
                     normalArtifacts,
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<AspectValue>of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
                     new DummyExecutor(fileSystem, rootDirectory),
                     builtTargets,
                     builtAspects,
@@ -2137,11 +2135,11 @@
                 builder.buildArtifacts(
                     reporter,
                     normalArtifacts,
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<ConfiguredTarget>of(),
-                    ImmutableSet.<AspectValue>of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
+                    ImmutableSet.of(),
                     new DummyExecutor(fileSystem, rootDirectory),
                     builtTargets,
                     builtAspects,
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 504898d..40892d5 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -54,7 +54,6 @@
 import com.google.devtools.build.lib.actions.util.DummyExecutor;
 import com.google.devtools.build.lib.actions.util.InjectedActionLookupKey;
 import com.google.devtools.build.lib.actions.util.TestAction;
-import com.google.devtools.build.lib.analysis.AspectValue;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.ServerDirectories;
@@ -113,7 +112,6 @@
 import java.io.IOException;
 import java.io.PrintStream;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -307,7 +305,7 @@
           Set<ConfiguredTarget> exclusiveTests,
           Set<ConfiguredTarget> targetsToBuild,
           Set<ConfiguredTarget> targetsToSkip,
-          Collection<AspectValue> aspects,
+          ImmutableSet<AspectKey> aspects,
           Executor executor,
           Set<ConfiguredTargetKey> builtTargets,
           Set<AspectKey> builtAspects,
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkActionProviderTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkActionProviderTest.java
index 7c2b1f5..3df8e8b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkActionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkActionProviderTest.java
@@ -60,7 +60,7 @@
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
 
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
@@ -122,7 +122,7 @@
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
 
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index f43ad96..88e98b3 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -45,6 +45,7 @@
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
 import com.google.devtools.build.lib.rules.objc.ObjcProtoProvider;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
 import com.google.devtools.build.lib.syntax.Depset;
 import com.google.devtools.build.lib.syntax.Sequence;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -103,7 +104,7 @@
     assertThat(getAspectDescriptions(analysisResult))
         .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
@@ -129,7 +130,7 @@
     assertThat(getAspectDescriptions(analysisResult))
         .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
@@ -160,7 +161,7 @@
         .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
 
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     StarlarkProvider.Key fooKey =
         new StarlarkProvider.Key(
@@ -175,11 +176,9 @@
 
   private Iterable<String> getAspectDescriptions(AnalysisResult analysisResult) {
     return transform(
-        analysisResult.getAspects(),
-        aspectValue ->
-            String.format(
-                "%s(%s)",
-                aspectValue.getConfiguredAspect().getName(), aspectValue.getLabel().toString()));
+        analysisResult.getAspectsMap().keySet(),
+        aspectKey ->
+            String.format("%s(%s)", aspectKey.getAspectClass().getName(), aspectKey.getLabel()));
   }
 
   @Test
@@ -241,7 +240,10 @@
 
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
-    AspectValue aspectValue = Iterables.getOnlyElement(analysisResult.getAspects());
+
+    AspectKey key = Iterables.getOnlyElement(analysisResult.getAspectsMap().keySet());
+    AspectValue aspectValue =
+        (AspectValue) skyframeExecutor.getEvaluatorForTesting().getExistingValue(key);
     AspectDefinition aspectDefinition = aspectValue.getAspect().getDefinition();
     assertThat(
             aspectDefinition
@@ -292,8 +294,7 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
     assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:xxx");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
     assertThat(configuredAspect).isNotNull();
     Object names = configuredAspect.get("target_labels");
     assertThat(names).isInstanceOf(Depset.class);
@@ -349,8 +350,7 @@
         ")");
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:yyy");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
     assertThat(configuredAspect).isNotNull();
     Object nameSet = configuredAspect.get("target_labels");
     ImmutableList<String> names =
@@ -424,8 +424,8 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
     assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:xxx");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    OutputGroupInfo outputGroupInfo = OutputGroupInfo.get(aspectValue.getConfiguredAspect());
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
+    OutputGroupInfo outputGroupInfo = OutputGroupInfo.get(configuredAspect);
 
     assertThat(outputGroupInfo).isNotNull();
     NestedSet<Artifact> names = outputGroupInfo.getOutputGroup("my_result");
@@ -457,8 +457,8 @@
                 analysisResult.getTargetsToBuild(),
                 configuredTarget -> configuredTarget.getLabel().toString()))
         .containsExactly("//test:xxx");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    OutputGroupInfo outputGroupInfo = OutputGroupInfo.get(aspectValue.getConfiguredAspect());
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
+    OutputGroupInfo outputGroupInfo = OutputGroupInfo.get(configuredAspect);
     assertThat(outputGroupInfo).isNotNull();
     NestedSet<Artifact> names = outputGroupInfo.getOutputGroup("my_result");
     assertThat(names.toList()).isNotEmpty();
@@ -723,8 +723,7 @@
     AnalysisResult result = update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:zzz.jar");
     assertThat(result.hasError()).isFalse();
     assertThat(
-            Iterables.getOnlyElement(result.getAspects())
-                .getConfiguredAspect()
+            Iterables.getOnlyElement(result.getAspectsMap().values())
                 .getProviders()
                 .getProviderCount())
         .isEqualTo(0);
@@ -845,8 +844,8 @@
             ImmutableList.of("//simple:print.bzl%print_expanded_location"),
             "//simple:concat_all_files");
     assertThat(analysisResult.hasError()).isFalse();
-    AspectValue value = Iterables.getOnlyElement(analysisResult.getAspects());
-    String result = (String) value.getConfiguredAspect().get("result");
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
+    String result = (String) configuredAspect.get("result");
 
     assertThat(result).isEqualTo("simple/afile");
   }
@@ -1651,8 +1650,7 @@
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:file.txt");
     assertThat(analysisResult.hasError()).isFalse();
     assertThat(
-            Iterables.getOnlyElement(analysisResult.getAspects())
-                .getConfiguredAspect()
+            Iterables.getOnlyElement(analysisResult.getAspectsMap().values())
                 .getProviders()
                 .getProviderCount())
         .isEqualTo(0);
@@ -1808,8 +1806,7 @@
         ")");
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:foo");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
     assertThat(configuredAspect).isNotNull();
     Object names = configuredAspect.get("target_labels");
     assertThat(names).isInstanceOf(Depset.class);
@@ -2251,7 +2248,7 @@
 
     AnalysisResult analysisResult = update(ImmutableList.of("//test:aspect.bzl%a3"), "//test:r2_1");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     StarlarkProvider.Key p3 =
         new StarlarkProvider.Key(Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "p3");
     StructImpl p3Provider = (StructImpl) configuredAspect.get(p3);
@@ -2354,7 +2351,7 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("//test:aspect.bzl%acollect"), "//test:baz");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     StarlarkProvider.Key pCollector =
         new StarlarkProvider.Key(
             Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "PCollector");
@@ -2398,7 +2395,7 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("//test:aspect.bzl%acollect"), "//test:baz");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     StarlarkProvider.Key pCollector =
         new StarlarkProvider.Key(
             Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "PCollector");
@@ -2453,7 +2450,7 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("//test:aspect.bzl%acollect"), "//test:quux");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     StarlarkProvider.Key pCollector =
         new StarlarkProvider.Key(
             Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "PCollector");
@@ -2494,7 +2491,7 @@
     AnalysisResult analysisResult =
         update(ImmutableList.of("//test:aspect.bzl%acollect"), "//test:baz");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
     StarlarkProvider.Key pCollector =
         new StarlarkProvider.Key(
             Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "PCollector");
@@ -2556,7 +2553,7 @@
             ImmutableList.of("test_skylark/top_level_stub.bzl%top_level_aspect"),
             "//bin:link_target");
     ConfiguredAspect configuredAspect =
-        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+        Iterables.getOnlyElement(analysisResult.getAspectsMap().values());
 
     ObjcProtoProvider objcProtoProvider =
         (ObjcProtoProvider) configuredAspect.get(ObjcProtoProvider.SKYLARK_CONSTRUCTOR.getKey());
@@ -2646,8 +2643,7 @@
         ")");
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:yyy");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
+    ConfiguredAspect configuredAspect = analysisResult.getAspectsMap().values().iterator().next();
     assertThat(configuredAspect).isNotNull();
 
     StarlarkProvider.Key myInfoKey =
@@ -2718,9 +2714,7 @@
     scratch.file("test/BUILD", "cc_library(", "     name = 'xxx',", ")");
     AnalysisResult analysisResult =
         update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
-    AspectValue aspectValue = analysisResult.getAspects().iterator().next();
-    ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
-    assertThat(configuredAspect).isNotNull();
+    assertThat(analysisResult.getAspectsMap().values().iterator().next()).isNotNull();
   }
 
   @Test