commit 03ca3edb7b7a257ac5fdb4431ca9e4f77709f1b5
author felly <felly@google.com> Thu May 16 13:32:43 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu May 16 13:33:51 2019 -0700
tree 1d149c0dd5bd04360e41c31ceafdd9e28f87a72a
parent fa388c30760b40021d94fb3e7a44b70c09f8456d

Package perf: Avoid another copy and loop iteration. Instead, defer loop iteration over the presumably smaller set of new input files and implicit test suites.

RELNOTES: None
PiperOrigin-RevId: 248589531

src/main/java/com/google/devtools/build/lib/packages/Package.java

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 6048ca6d..77b2c8e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1425,16 +1425,22 @@
 
       // The Iterable returned by getTargets is sorted, so when we build up the list of tests by
       // processing it in order below, that list will be sorted too.
-      Iterable<Rule> sortedRules = ImmutableList.copyOf(getTargets(Rule.class));
 
       List<Label> sortedTests = new ArrayList<>();
-      for (final Rule rule : sortedRules) {
+      List<Rule> implicitTestSuites = new ArrayList<>();
+      Map<Label, InputFile> newInputFiles = new HashMap<>();
+      for (final Rule rule : getTargets(Rule.class)) {
         if (discoverAssumedInputFiles) {
           // All labels mentioned in a rule that refer to an unknown target in the
           // current package are assumed to be InputFiles, so let's create them:
           for (AttributeMap.DepEdge depEdge : AggregatingAttributeMapper.of(rule).visitLabels()) {
-            createInputFileMaybe(
-                depEdge.getLabel(), rule.getAttributeLocation(depEdge.getAttribute().getName()));
+            InputFile inputFile =
+                createInputFileMaybe(
+                    depEdge.getLabel(),
+                    rule.getAttributeLocation(depEdge.getAttribute().getName()));
+            if (inputFile != null && !newInputFiles.containsKey(depEdge.getLabel())) {
+              newInputFiles.put(depEdge.getLabel(), inputFile);
+            }
           }
         }
 
@@ -1446,14 +1452,21 @@
         if (TargetUtils.isTestRule(rule) && !TargetUtils.hasManualTag(rule)) {
           sortedTests.add(rule.getLabel());
         }
-      }
-      for (Rule rule : sortedRules) {
+
         AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
         if (rule.getRuleClass().equals("test_suite")
             && attributes.get("tests", BuildType.LABEL_LIST).isEmpty()) {
-          rule.setAttributeValueByName("$implicit_tests", sortedTests);
+          implicitTestSuites.add(rule);
         }
       }
+
+      for (InputFile inputFile : newInputFiles.values()) {
+        addInputFile(inputFile);
+      }
+
+      for (Rule rule : implicitTestSuites) {
+        rule.setAttributeValueByName("$implicit_tests", sortedTests);
+      }
       return this;
     }
 
@@ -1508,20 +1521,24 @@
     }
 
     /**
-     * If "label" refers to a non-existent target in the current package, create
-     * an InputFile target.
+     * If "label" refers to a non-existent target in the current package, create an InputFile
+     * target.
      */
-    private void createInputFileMaybe(Label label, Location location) {
+    private InputFile createInputFileMaybe(Label label, Location location) {
       if (label != null && label.getPackageIdentifier().equals(pkg.getPackageIdentifier())) {
         if (!targets.containsKey(label.getName())) {
-          addInputFile(label, location);
+          return new InputFile(pkg, label, location);
         }
       }
+      return null;
     }
 
     private InputFile addInputFile(Label label, Location location) {
-      InputFile inputFile = new InputFile(pkg, label, location);
-      Target prev = targets.put(label.getName(), inputFile);
+      return addInputFile(new InputFile(pkg, label, location));
+    }
+
+    private InputFile addInputFile(InputFile inputFile) {
+      Target prev = targets.put(inputFile.getLabel().getName(), inputFile);
       Preconditions.checkState(prev == null);
       return inputFile;
     }