commit e4e585faa51a3bfe3d0171c7e00840e78364bdb0
author adonovan <adonovan@google.com> Tue Jun 30 06:38:10 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Jun 30 06:39:33 2020 -0700
tree 27e048d60e140d3e119570b40834c46236f23da5
parent b133fe37dded626742ad04fdf3c33244cebcd05d

bazel packages: reduce generation of garbage InputFiles

...by inlining createInputFileMaybe and simplifying.

PiperOrigin-RevId: 319010691

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 0e77871..8fa17ca 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
@@ -1503,16 +1503,24 @@
       buildFile = (InputFile) Preconditions.checkNotNull(targets.get(buildFileLabel.getName()));
 
       List<Label> tests = new ArrayList<>();
-      List<Rule> implicitTestSuiteRuleInstances = new ArrayList<>();
-      Map<Label, InputFile> newInputFiles = new HashMap<>();
+      Map<String, 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()) {
-            InputFile inputFile = createInputFileMaybe(depEdge.getLabel(), rule.getLocation());
-            if (inputFile != null && !newInputFiles.containsKey(depEdge.getLabel())) {
-              newInputFiles.put(depEdge.getLabel(), inputFile);
+          // All labels mentioned by a rule that refer to an unknown target in the
+          // current package are assumed to be InputFiles, so let's create them.
+          // (We add them to a temporary map while we are iterating over this.targets.)
+          for (AttributeMap.DepEdge edge : AggregatingAttributeMapper.of(rule).visitLabels()) {
+            Label label = edge.getLabel();
+            if (label.getPackageIdentifier().equals(pkg.getPackageIdentifier())
+                && !targets.containsKey(label.getName())
+                && !newInputFiles.containsKey(label.getName())) {
+              Location loc = rule.getLocation();
+              newInputFiles.put(
+                  label.getName(),
+                  noImplicitFileExport
+                      ? new InputFile(
+                          pkg, label, loc, ConstantRuleVisibility.PRIVATE, License.NO_LICENSE)
+                      : new InputFile(pkg, label, loc));
             }
           }
         }
@@ -1533,8 +1541,8 @@
       Collections.sort(tests); // (for determinism)
       this.testSuiteImplicitTests.addAll(tests);
 
-      for (InputFile inputFile : newInputFiles.values()) {
-        addInputFile(inputFile);
+      for (InputFile file : newInputFiles.values()) {
+        addInputFile(file);
       }
 
       return this;
@@ -1590,22 +1598,6 @@
       return finishBuild();
     }
 
-    /**
-     * If "label" refers to a non-existent target in the current package, create an InputFile
-     * target.
-     */
-    private InputFile createInputFileMaybe(Label label, Location location) {
-      if (label != null && label.getPackageIdentifier().equals(pkg.getPackageIdentifier())) {
-        if (!targets.containsKey(label.getName())) {
-          return noImplicitFileExport
-              ? new InputFile(
-                  pkg, label, location, ConstantRuleVisibility.PRIVATE, License.NO_LICENSE)
-              : new InputFile(pkg, label, location);
-        }
-      }
-      return null;
-    }
-
     private InputFile addInputFile(Label label, Location location) {
       return addInputFile(new InputFile(pkg, label, location));
     }