Use Set<Target> instead of ResolvedTargets<Target> when saving resolved target patterns in query.

We never use anything but the getTargets() field, so it's useless to store other things.

--
MOS_MIGRATED_REVID=100846573

3 files changed, 30 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
index 779ede8210..81c8f33670 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java
@@ -16,8 +16,8 @@ package com.google.devtools.build.lib.query2;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
-import com.google.devtools.build.lib.cmdline.ResolvedTargets;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.events.ErrorSensingEventHandler;
 import com.google.devtools.build.lib.events.Event;
@@ -55,7 +55,7 @@ import javax.annotation.Nullable;
 public abstract class AbstractBlazeQueryEnvironment<T> implements QueryEnvironment<T> {
   protected final ErrorSensingEventHandler eventHandler;
   private final Map<String, Set<T>> letBindings = new HashMap<>();
-  protected final Map<String, ResolvedTargets<Target>> resolvedTargetPatterns = new HashMap<>();
+  protected final Map<String, Set<Target>> resolvedTargetPatterns = new HashMap<>();
   protected final boolean keepGoing;
   protected final boolean strictScope;
 
@@ -225,15 +225,16 @@ public abstract class AbstractBlazeQueryEnvironment<T> implements QueryEnvironme
         resolvedTargetPatterns.putAll(preloadOrThrow(caller, ImmutableList.of(pattern)));
       } catch (TargetParsingException e) {
         // Will skip the target and keep going if -k is specified.
-        resolvedTargetPatterns.put(pattern, ResolvedTargets.<Target>empty());
+        resolvedTargetPatterns.put(pattern, ImmutableSet.<Target>of());
         reportBuildFileError(caller, e.getMessage());
       }
     }
     return getTargetsMatchingPattern(caller, pattern);
   }
 
-  protected abstract Map<String, ResolvedTargets<Target>> preloadOrThrow(QueryExpression caller,
-      Collection<String> patterns) throws QueryException, TargetParsingException;
+  protected abstract Map<String, Set<Target>> preloadOrThrow(
+      QueryExpression caller, Collection<String> patterns)
+      throws QueryException, TargetParsingException;
 
   @Override
   public boolean isSettingEnabled(Setting setting) {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
index db9d637257..208087f806 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java
@@ -13,10 +13,12 @@
 // limitations under the License.
 package com.google.devtools.build.lib.query2;
 
+import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.cmdline.ResolvedTargets;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -112,7 +114,7 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     // We can safely ignore the boolean error flag. The evaluateQuery() method above wraps the
     // entire query computation in an error sensor.
 
-    Set<Target> targets = new LinkedHashSet<>(resolvedTargetPatterns.get(pattern).getTargets());
+    Set<Target> targets = new LinkedHashSet<>(resolvedTargetPatterns.get(pattern));
 
     // Sets.filter would be more convenient here, but can't deal with exceptions.
     Iterator<Target> targetIterator = targets.iterator();
@@ -345,13 +347,22 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
     return dependentFiles;
   }
 
-  protected Map<String, ResolvedTargets<Target>> preloadOrThrow(QueryExpression caller,
-      Collection<String> patterns) throws TargetParsingException {
+  private static final Function<ResolvedTargets<Target>, Set<Target>> RESOLVED_TARGETS_TO_TARGETS =
+      new Function<ResolvedTargets<Target>, Set<Target>>() {
+        @Override
+        public Set<Target> apply(ResolvedTargets<Target> resolvedTargets) {
+          return resolvedTargets.getTargets();
+        }
+      };
+
+  protected Map<String, Set<Target>> preloadOrThrow(
+      QueryExpression caller, Collection<String> patterns) throws TargetParsingException {
     try {
       // Note that this may throw a RuntimeException if deps are missing in Skyframe and this is
       // being called from within a SkyFunction.
-      return targetPatternEvaluator.preloadTargetPatterns(
-          eventHandler, patterns, keepGoing);
+      return Maps.transformValues(
+          targetPatternEvaluator.preloadTargetPatterns(eventHandler, patterns, keepGoing),
+          RESOLVED_TARGETS_TO_TARGETS);
     } catch (InterruptedException e) {
       // TODO(bazel-team): Propagate the InterruptedException from here [skyframe-loading].
       throw new TargetParsingException("interrupted");
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index cc2068ca26..db8ed2da76 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -267,7 +267,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
   @Override
   public Set<Target> getTargetsMatchingPattern(QueryExpression owner, String pattern)
       throws QueryException {
-    Set<Target> targets = new LinkedHashSet<>(resolvedTargetPatterns.get(pattern).getTargets());
+    Set<Target> targets = new LinkedHashSet<>(resolvedTargetPatterns.get(pattern));
 
     // Sets.filter would be more convenient here, but can't deal with exceptions.
     Iterator<Target> targetIterator = targets.iterator();
@@ -367,11 +367,12 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
   }
 
   @Override
-  protected Map<String, ResolvedTargets<Target>> preloadOrThrow(QueryExpression caller,
-      Collection<String> patterns) throws QueryException, TargetParsingException {
+  protected Map<String, Set<Target>> preloadOrThrow(
+      QueryExpression caller, Collection<String> patterns)
+      throws QueryException, TargetParsingException {
     GraphBackedRecursivePackageProvider provider =
         new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys);
-    Map<String, ResolvedTargets<Target>> result = Maps.newHashMapWithExpectedSize(patterns.size());
+    Map<String, Set<Target>> result = Maps.newHashMapWithExpectedSize(patterns.size());
     for (String pattern : patterns) {
       SkyKey patternKey = TargetPatternValue.key(pattern,
           TargetPatternEvaluator.DEFAULT_FILTERING_POLICY, parserPrefix);
@@ -387,7 +388,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
           ResolvedTargets.Builder<Target> targetsBuilder = ResolvedTargets.builder();
           targetsBuilder.addAll(makeTargetsFromLabels(value.getTargets().getTargets()));
           targetsBuilder.removeAll(makeTargetsFromLabels(value.getTargets().getFilteredTargets()));
-          result.put(pattern, targetsBuilder.build());
+          result.put(pattern, targetsBuilder.build().getTargets());
         } else {
           // Because the graph was always initialized via a keep_going build, we know that the
           // exception stored here must be a TargetParsingException. Thus the comment in
@@ -405,7 +406,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
                 targetPatternKey.getPolicy(), pkgPath);
         TargetPattern parsedPattern = targetPatternKey.getParsedPattern();
         try {
-          result.put(pattern, parsedPattern.eval(resolver));
+          result.put(pattern, parsedPattern.eval(resolver).getTargets());
         } catch (TargetParsingException e) {
           targetParsingException = e;
         } catch (InterruptedException e) {
@@ -419,7 +420,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
         } else {
           eventHandler.handle(Event.error("Evaluation of query \"" + caller + "\" failed: "
               + targetParsingException.getMessage()));
-          result.put(pattern, ResolvedTargets.<Target>builder().setError().build());
+          result.put(pattern, ImmutableSet.<Target>of());
         }
       }
     }