Skip to content
Permalink
Browse files

Ensure only one ComponentPath instance exists for each logical Compon…

…entPath (per dagger.model.BindingGraph)

This ensures that ComponentPaths used in BindingGraph.Nodes are all based on instance equality instead of checking their composing lists.

This has a decrease of almost 3s/build for a large internal component, or ~11% of Dagger processing time. With AOT turned on, it saves 6s/build, or 14% of Dagger processing time.

RELNOTES=Build performance improvements for large components

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=232760752
  • Loading branch information
ronshapiro committed Feb 6, 2019
1 parent 1d5d829 commit 6cb9f6bc69ac995ac09ecd3c01f1eb9efd90e547
Showing with 28 additions and 11 deletions.
  1. +17 −10 java/dagger/internal/codegen/ComponentTreeTraverser.java
  2. +11 −1 java/dagger/model/ComponentPath.java
@@ -19,6 +19,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Verify.verify;
import static dagger.internal.codegen.DaggerStreams.toImmutableList;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterators;
@@ -42,6 +43,9 @@
/** The path from the root graph to the currently visited graph. */
private final Deque<BindingGraph> bindingGraphPath = new ArrayDeque<>();

/** The {@link ComponentPath} for each component in {@link #bindingGraphPath}. */
private final Deque<ComponentPath> componentPaths = new ArrayDeque<>();

/** Constructs a traverser for a root (component, not subcomponent) binding graph. */
public ComponentTreeTraverser(BindingGraph rootGraph, CompilerOptions compilerOptions) {
checkArgument(
@@ -50,6 +54,7 @@ public ComponentTreeTraverser(BindingGraph rootGraph, CompilerOptions compilerOp
"only root graphs can be traversed, not %s",
rootGraph.componentTypeElement().getQualifiedName());
bindingGraphPath.add(rootGraph);
componentPaths.add(ComponentPath.create(ImmutableList.of(rootGraph.componentTypeElement())));
}

/**
@@ -59,6 +64,7 @@ public ComponentTreeTraverser(BindingGraph rootGraph, CompilerOptions compilerOp
*/
public final void traverseComponents() {
checkState(bindingGraphPath.size() == 1);
checkState(componentPaths.size() == 1);
visitComponent(bindingGraphPath.getFirst());
}

@@ -101,10 +107,17 @@ protected void visitComponent(BindingGraph graph) {

for (BindingGraph child : graph.subgraphs()) {
bindingGraphPath.addLast(child);
ComponentPath childPath =
ComponentPath.create(
bindingGraphPath.stream()
.map(BindingGraph::componentTypeElement)
.collect(toImmutableList()));
componentPaths.addLast(childPath);
try {
visitComponent(child);
} finally {
verify(bindingGraphPath.removeLast().equals(child));
verify(componentPaths.removeLast().equals(childPath));
}
}
}
@@ -140,23 +153,17 @@ protected void visitEntryPoint(DependencyRequest entryPoint, BindingGraph graph)
* component.
*/
protected final ComponentPath componentPath() {
ImmutableList.Builder<TypeElement> path = ImmutableList.builder();
for (BindingGraph graph : bindingGraphPath) {
path.add(graph.componentTypeElement());
}
return ComponentPath.create(path.build());
return componentPaths.getLast();
}

/**
* Returns the subpath from the root component to the matching {@code ancestor} of the current
* component.
*/
protected final ComponentPath pathFromRootToAncestor(TypeElement ancestor) {
ImmutableList.Builder<TypeElement> path = ImmutableList.builder();
for (TypeElement component : componentPath().components()) {
path.add(component);
if (component.equals(ancestor)) {
return ComponentPath.create(path.build());
for (ComponentPath componentPath : componentPaths) {
if (componentPath.currentComponent().equals(ancestor)) {
return componentPath;
}
}
throw new IllegalArgumentException(
@@ -21,6 +21,7 @@
import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.collect.ImmutableList;
import javax.lang.model.element.TypeElement;

@@ -47,7 +48,8 @@ public final TypeElement rootComponent() {
}

/** Returns the component at the end of the path. */
public final TypeElement currentComponent() {
@Memoized
public TypeElement currentComponent() {
return getLast(components());
}

@@ -66,6 +68,7 @@ public final TypeElement parentComponent() {
*
* @throws IllegalStateException if the current graph is the {@linkplain #atRoot() root component}
*/
// TODO(ronshapiro): consider memoizing this
public final ComponentPath parent() {
checkState(!atRoot());
return create(components().subList(0, components().size() - 1));
@@ -88,4 +91,11 @@ public final boolean atRoot() {
public final String toString() {
return components().stream().map(TypeElement::getQualifiedName).collect(joining(""));
}

@Memoized
@Override
public abstract int hashCode();

@Override
public abstract boolean equals(Object obj);
}

0 comments on commit 6cb9f6b

Please sign in to comment.
You can’t perform that action at this time.