Rephrase AbstractAttributeMapper#visitLabels such that we can avoid creating a temporary Type.LabelVisitor instance per Attribute being visited. Instead, we can now create one temporary object per visitation. Getting rid of this dimension of scaling reduces the amount of garbage created.

RELNOTES: None

PiperOrigin-RevId: 152161836

1 files changed, 24 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
index e3a390e958..73d786bf5a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.packages.Attribute.ComputationLimiter;
 import com.google.devtools.build.lib.packages.BuildType.Selector;
 import com.google.devtools.build.lib.packages.BuildType.SelectorList;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelVisitor;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -71,13 +72,13 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
    * path whenever actual value iteration isn't specifically needed.
    */
   @Override
-  protected void visitLabels(Attribute attribute, AcceptsLabelAttribute observer)
+  protected void visitLabels(Attribute attribute, Type.LabelVisitor<Attribute> visitor)
       throws InterruptedException {
-    visitLabels(attribute, true, observer);
+    visitLabels(attribute, true, visitor);
   }
 
   private void visitLabels(
-      final Attribute attribute, boolean includeSelectKeys, final AcceptsLabelAttribute observer)
+      Attribute attribute, boolean includeSelectKeys, Type.LabelVisitor<Attribute> visitor)
       throws InterruptedException {
     Type<?> type = attribute.getType();
     SelectorList<?> selectorList = getSelectorList(attribute.getName(), type);
@@ -87,39 +88,22 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
         // (computed) values and look for labels.
         for (Object value : visitAttribute(attribute.getName(), attribute.getType())) {
           if (value != null) {
-            type.visitLabels(new Type.LabelVisitor() {
-              @Override
-              public void visit(@Nullable Label label) throws InterruptedException {
-                if (label != null) {
-                  observer.acceptLabelAttribute(
-                      getLabel().resolveRepositoryRelative(label), attribute);
-                }
-              }
-            }, value);
+            type.visitLabels(visitor, value, attribute);
           }
         }
       } else {
-        super.visitLabels(attribute, observer);
+        super.visitLabels(attribute, visitor);
       }
     } else {
       for (Selector<?> selector : selectorList.getSelectors()) {
         for (Map.Entry<Label, ?> selectorEntry : selector.getEntries().entrySet()) {
           if (includeSelectKeys && !BuildType.Selector.isReservedLabel(selectorEntry.getKey())) {
-            observer.acceptLabelAttribute(
-                getLabel().resolveRepositoryRelative(selectorEntry.getKey()), attribute);
+            visitor.visit(selectorEntry.getKey(), attribute);
           }
           Object value = selector.isValueSet(selectorEntry.getKey())
               ? selectorEntry.getValue()
               : attribute.getDefaultValue(null);
-          type.visitLabels(new Type.LabelVisitor() {
-            @Override
-            public void visit(@Nullable Label label) throws InterruptedException {
-              if (label != null) {
-                observer.acceptLabelAttribute(
-                    getLabel().resolveRepositoryRelative(label), attribute);
-              }
-            }
-          }, value);
+          type.visitLabels(visitor, value, attribute);
         }
       }
     }
@@ -133,10 +117,12 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
   public Set<Label> getReachableLabels(String attributeName, boolean includeSelectKeys)
       throws InterruptedException {
     final ImmutableSet.Builder<Label> builder = ImmutableSet.<Label>builder();
-    visitLabels(getAttributeDefinition(attributeName), includeSelectKeys,
-        new AcceptsLabelAttribute() {
+    visitLabels(
+        getAttributeDefinition(attributeName),
+        includeSelectKeys,
+        new LabelVisitor<Attribute>() {
           @Override
-          public void acceptLabelAttribute(Label label, Attribute attribute) {
+          public void visit(Label label, Attribute attribute) {
             builder.add(label);
           }
         });
@@ -538,14 +524,17 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
   private static ImmutableList<Label> extractLabels(Type<?> type, Object value) {
     try {
       final ImmutableList.Builder<Label> result = ImmutableList.builder();
-      type.visitLabels(new Type.LabelVisitor() {
-        @Override
-        public void visit(@Nullable Label label) {
-          if (label != null) {
-            result.add(label);
-          }
-        }
-      }, value);
+      type.visitLabels(
+          new Type.LabelVisitor<Object>() {
+            @Override
+            public void visit(@Nullable Label label, Object dummy) {
+              if (label != null) {
+                result.add(label);
+              }
+            }
+          },
+          value,
+          /*context=*/ null);
       return result.build();
     } catch (InterruptedException e) {
       throw new IllegalStateException("Unexpected InterruptedException", e);