Optimize how null configurations get created and add test infrastructure for Bazel's dep configuration creation logic.

This essentially implements the following TODOs:

https://github.com/bazelbuild/bazel/blob/bc6045dcc8fa33d4241d231138020ac4bdecc14f/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java#L599
https://github.com/bazelbuild/bazel/blob/bc6045dcc8fa33d4241d231138020ac4bdecc14f/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java#L42

--
MOS_MIGRATED_REVID=134607049

3 files changed, 11 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 7b89d6f142..e00a95edef 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -1742,7 +1742,13 @@ public final class BuildConfiguration {
     public Iterable<Dependency> getDependencies(
         Label label, ImmutableSet<AspectDescriptor> aspects) {
       return ImmutableList.of(
-          Dependency.withTransitionAndAspects(label, currentTransition, aspects));
+          isNull()
+              // We can trivially set the final value for null-configured targets now. This saves
+              // us from having to recreate a new Dependency object for the final value later. Since
+              // there are lots of null-configured targets (e.g. all source files), this can add up
+              // over the course of a build.
+              ? Dependency.withNullConfiguration(label)
+              : Dependency.withTransitionAndAspects(label, currentTransition, aspects));
     }
   }
 
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 4868332f73..862a5a95ef 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
@@ -474,12 +474,13 @@ final class ConfiguredTargetFunction implements SkyFunction {
       // Certain targets (like output files) trivially re-use their input configuration. Likewise,
       // deps with null configurations (e.g. source files), can be trivially computed. So we skip
       // all logic in this method for these cases and just reinsert their original configurations
-      // back at the end.
+      // back at the end (note that null-configured targets will have a static
+      // NullConfigurationDependency instead of dynamic
+      // Dependency(label, transition=Attribute.Configuration.Transition.NULL)).
       //
       // A *lot* of targets have null deps, so this produces real savings. Profiling tests over a
       // simple cc_binary show this saves ~1% of total analysis phase time.
-      if (dep.hasStaticConfiguration()
-          || dep.getTransition() == Attribute.ConfigurationTransition.NULL) {
+      if (dep.hasStaticConfiguration()) {
         continue;
       }
 
@@ -595,12 +596,6 @@ final class ConfiguredTargetFunction implements SkyFunction {
           new AttributeAndLabel(depsEntry.getKey(), depsEntry.getValue().getLabel());
       if (depsEntry.getValue().hasStaticConfiguration()) {
         result.put(attrAndLabel.attribute, depsEntry.getValue());
-      } else if (depsEntry.getValue().getTransition() == Attribute.ConfigurationTransition.NULL) {
-        // TODO(gregce): explore re-using the original Dependency (which would have to become a
-        // null-configured static Dependency instead of a dynamic Dependency with a NULL transition)
-        // instead of needlessly recreating the object here. Some memory/performance profiling
-        // would help determine the right course.
-        result.put(attrAndLabel.attribute, Dependency.withNullConfiguration(attrAndLabel.label));
       } else {
         Collection<Dependency> trimmedAttrDeps = trimmedDeps.get(attrAndLabel);
         Verify.verify(!trimmedAttrDeps.isEmpty());
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 8f91b505cf..804b950d4d 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
@@ -1257,8 +1257,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     for (Dependency key : keys) {
       if (key.hasStaticConfiguration()) {
         builder.put(key, key.getConfiguration());
-      } else if (key.getTransition() == Attribute.ConfigurationTransition.NULL) {
-        builder.put(key, null);
       } else if (useUntrimmedDynamicConfigs(fromOptions)) {
         fragmentsMap.put(key.getLabel(), allFragments);
       } else {