Store a binding graph's subgraphs as a list instead of a set

BindingGraph.hashCode() is extremely slow as has already been discovered, but even memoizing the value doesn't do much value in cases where it is unlikely for the hash code to be computed and it is only computed once.

It should be impossible to try and resolve the same child graph twice for the same graph, and I'd argue that doing so represents a deeper bug in the binding graph resolution algorithm. Not catching the bug will likely result in Dagger attempting to generate the same component implementation twice, in turn causing a compiler error and a better indication of the bug than what we have by keeping a set.

We never use this set for set-like properties. We only use it for iteration purposes.

This is particularly devastating in ahead-of-time components, where we create N BindingGraph instances for a particular component in a hierarchy, where N is the depth in the hierarchy. Profiling a large component internally with AOT turned on showed BindingGraph.hashCode() taking 7.3s, or 2.8% of the entire compilation (12.4% of total annotation processing).

RELNOTES=Peformance improvements in annotation processing

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=225387291

java/dagger/internal/codegen/BindingGraph.java

@@ -23,12 +23,17 @@
 import com.google.auto.value.AutoValue;
 import com.google.auto.value.extension.memoized.Memoized;
 import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
 import com.google.common.graph.Traverser;
 import dagger.Subcomponent;
 import dagger.model.Key;
 import dagger.model.RequestKind;
+import java.util.Collection;
+import java.util.Map;
 import java.util.Optional;
 import java.util.stream.StreamSupport;
 import javax.lang.model.element.ExecutableElement;
@@ -76,7 +81,7 @@ ImmutableSet<ResolvedBindings> resolvedBindings() {
         .build();
   }
 
-  abstract ImmutableSet<BindingGraph> subgraphs();
+  abstract ImmutableList<BindingGraph> subgraphs();
 
   /**
    * The type that defines the component for this graph.
@@ -226,9 +231,10 @@ static BindingGraph create(
       ComponentDescriptor componentDescriptor,
       ImmutableMap<Key, ResolvedBindings> resolvedContributionBindingsMap,
       ImmutableMap<Key, ResolvedBindings> resolvedMembersInjectionBindings,
-      ImmutableSet<BindingGraph> subgraphs,
+      ImmutableList<BindingGraph> subgraphs,
       ImmutableSet<ModuleDescriptor> ownedModules,
       Optional<ExecutableElement> factoryMethod) {
+    checkForDuplicates(subgraphs);
     return new AutoValue_BindingGraph(
         componentDescriptor,
         resolvedContributionBindingsMap,
@@ -237,4 +243,14 @@ static BindingGraph create(
         ownedModules,
         factoryMethod);
   }
+
+  private static final void checkForDuplicates(Iterable<BindingGraph> graphs) {
+    Map<TypeElement, Collection<BindingGraph>> duplicateGraphs =
+        Maps.filterValues(
+            Multimaps.index(graphs, graph -> graph.componentDescriptor().typeElement()).asMap(),
+            overlapping -> overlapping.size() > 1);
+    if (!duplicateGraphs.isEmpty()) {
+      throw new IllegalArgumentException("Expected no duplicates: " + duplicateGraphs);
+    }
+  }
 }

java/dagger/internal/codegen/BindingGraphFactory.java

@@ -188,7 +188,7 @@ private BindingGraph create(
     // done in a queue since resolving one subcomponent might resolve a key for a subcomponent
     // from a parent graph. This is done until no more new subcomponents are resolved.
     Set<ComponentDescriptor> resolvedSubcomponents = new HashSet<>();
-    ImmutableSet.Builder<BindingGraph> subgraphs = ImmutableSet.builder();
+    ImmutableList.Builder<BindingGraph> subgraphs = ImmutableList.builder();
     for (ComponentDescriptor subcomponent :
         Iterables.consumingIterable(requestResolver.subcomponentsToResolve)) {
       if (resolvedSubcomponents.add(subcomponent)) {