Skip to content

Commit

Permalink
Optimize BindingGraph.create() by indexing all binding nodes by their…
Browse files Browse the repository at this point in the history
… component path instead of traversing the full network for each subcomponent.

The traversals maintain sets of visited nodes, which aren't great for hashing. Network.successors() is also not well optimized, so repeating that is costly.

RELNOTES=Build performance improvements
PiperOrigin-RevId: 361390045
  • Loading branch information
ronshapiro authored and Dagger Team committed Mar 7, 2021
1 parent 1caa7f0 commit 47123ec
Showing 1 changed file with 20 additions and 20 deletions.
40 changes: 20 additions & 20 deletions java/dagger/internal/codegen/binding/BindingGraph.java
Expand Up @@ -16,6 +16,7 @@

package dagger.internal.codegen.binding;

import static com.google.common.collect.Iterables.transform;
import static dagger.internal.codegen.extension.DaggerCollectors.toOptional;
import static dagger.internal.codegen.extension.DaggerStreams.presentValues;
import static dagger.internal.codegen.extension.DaggerStreams.stream;
Expand All @@ -26,11 +27,12 @@
import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Multimaps;
import com.google.common.collect.Sets;
import com.google.common.graph.Graphs;
import com.google.common.graph.ImmutableNetwork;
import com.google.common.graph.Traverser;
import dagger.model.BindingGraph.ChildFactoryMethodEdge;
Expand All @@ -39,7 +41,9 @@
import dagger.model.BindingGraph.Node;
import dagger.model.ComponentPath;
import dagger.model.Key;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.lang.model.element.ExecutableElement;
Expand Down Expand Up @@ -104,6 +108,15 @@ ImmutableSet<ComponentNode> subcomponentNodes(ComponentNode componentNode) {
public ImmutableSetMultimap<Class<? extends Node>, ? extends Node> nodesByClass() {
return super.nodesByClass();
}

/**
* Returns an index of each {@link BindingNode} by its {@link ComponentPath}. Accessing this for
* a component and its parent components is faster than doing a graph traversal.
*/
@Memoized
ImmutableListMultimap<ComponentPath, BindingNode> bindingsByComponent() {
return Multimaps.index(transform(bindings(), BindingNode.class::cast), Node::componentPath);
}
}

static BindingGraph create(
Expand All @@ -115,12 +128,12 @@ private static BindingGraph create(
Optional<BindingGraph> parent,
ComponentNode componentNode,
TopLevelBindingGraph topLevelBindingGraph) {
ImmutableSet<BindingNode> reachableBindingNodes =
Graphs.reachableNodes(topLevelBindingGraph.network().asGraph(), componentNode).stream()
.filter(node -> node instanceof BindingNode)
.filter(node -> isSubpath(componentNode.componentPath(), node.componentPath()))
.map(node -> (BindingNode) node)
.collect(toImmutableSet());
List<BindingNode> reachableBindingNodes = new ArrayList<>();
for (ComponentPath path = componentNode.componentPath();
!path.components().isEmpty();
path = ComponentPath.create(path.components().subList(0, path.components().size() - 1))) {
reachableBindingNodes.addAll(topLevelBindingGraph.bindingsByComponent().get(path));
}

// Construct the maps of the ContributionBindings and MembersInjectionBindings.
Map<Key, BindingNode> contributionBindings = new HashMap<>();
Expand Down Expand Up @@ -331,17 +344,4 @@ public ImmutableSet<BindingNode> bindingNodes() {
.addAll(membersInjectionBindings.values())
.build();
}

// TODO(bcorso): Move this to ComponentPath
private static boolean isSubpath(ComponentPath path, ComponentPath subpath) {
if (path.components().size() < subpath.components().size()) {
return false;
}
for (int i = 0; i < subpath.components().size(); i++) {
if (!path.components().get(i).equals(subpath.components().get(i))) {
return false;
}
}
return true;
}
}

0 comments on commit 47123ec

Please sign in to comment.