Implement label visitation using visitor

As opposed to building up a collection. These collections, and all their iterators,
add up creating a lot of garbage. This saves us at least an ImmutableList + Iterator
per label.

--
MOS_MIGRATED_REVID=133754998

1 files changed, 40 insertions, 10 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 abaccccdff..39dec2719f 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
@@ -105,7 +105,7 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
   }
 
   private void visitLabels(
-      Attribute attribute, boolean includeSelectKeys, AcceptsLabelAttribute observer)
+      final Attribute attribute, boolean includeSelectKeys, final AcceptsLabelAttribute observer)
       throws InterruptedException {
     Type<?> type = attribute.getType();
     SelectorList<?> selectorList = getSelectorList(attribute.getName(), type);
@@ -115,9 +115,15 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
         // (computed) values and look for labels.
         for (Object value : visitAttribute(attribute.getName(), attribute.getType())) {
           if (value != null) {
-            for (Label label : extractLabels(type, value)) {
-              observer.acceptLabelAttribute(getLabel().resolveRepositoryRelative(label), attribute);
-            }
+            type.visitLabels(new Type.LabelVisitor() {
+              @Override
+              public void visit(@Nullable Object label) throws InterruptedException {
+                if (label != null) {
+                  observer.acceptLabelAttribute(
+                      getLabel().resolveRepositoryRelative((Label) label), attribute);
+                }
+              }
+            }, value);
           }
         }
       } else {
@@ -133,9 +139,15 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
           Object value = selector.isValueSet(selectorEntry.getKey())
               ? selectorEntry.getValue()
               : attribute.getDefaultValue(null);
-          for (Label label : extractLabels(type, value)) {
-            observer.acceptLabelAttribute(getLabel().resolveRepositoryRelative(label), attribute);
-          }
+          type.visitLabels(new Type.LabelVisitor() {
+            @Override
+            public void visit(@Nullable Object label) throws InterruptedException {
+              if (label != null) {
+                observer.acceptLabelAttribute(
+                    getLabel().resolveRepositoryRelative((Label) label), attribute);
+              }
+            }
+          }, value);
         }
       }
     }
@@ -178,8 +190,9 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
       //  3) "attr = select({...})". With just a single select, visitAttribute runs efficiently.
       for (Object value : visitAttribute(attrName, attrType)) {
         if (value != null) {
-          duplicates.addAll(CollectionUtils.duplicatedElementsOf(
-              ImmutableList.copyOf(extractLabels(attrType, value))));
+          // TODO(bazel-team): Calculate duplicates directly using attrType.visitLabels in order to
+          // avoid intermediate collections here.
+          duplicates.addAll(CollectionUtils.duplicatedElementsOf(extractLabels(attrType, value)));
         }
       }
     } else {
@@ -194,7 +207,7 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper {
         // they're in different selector paths (since only one path can actually get chosen).
         Set<Label> selectorLabels = new LinkedHashSet<>();
         for (Object selectorValue : selector.getEntries().values()) {
-          Iterable<Label> labelsInSelectorValue = extractLabels(attrType, selectorValue);
+          List<Label> labelsInSelectorValue = extractLabels(attrType, selectorValue);
           // Duplicates within a single path are not okay.
           duplicates.addAll(CollectionUtils.duplicatedElementsOf(labelsInSelectorValue));
           Iterables.addAll(selectorLabels, labelsInSelectorValue);
@@ -610,4 +623,21 @@ 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 Object object) {
+          if (object != null) {
+            result.add((Label) object);
+          }
+        }
+      }, value);
+      return result.build();
+    } catch (InterruptedException e) {
+      throw new IllegalStateException("Unexpected InterruptedException", e);
+    }
+  }
 }