Skip to content

Commit

Permalink
[Dagger Optimization]: Avoid copying bindings for each parent compone…
Browse files Browse the repository at this point in the history
…nt when creating our `BindingGraph`.

This CL replaces `LegacyBindingGraph` with a new class `BindingGraphFactory.LegacyBindingGraph` which is nested in `BindingGraphFactory` so that it can delegate directly to its `BindingGraphFactory.Resolver`. This allows us to just reuse the resolver under the hood and to implement it in a way that avoids copying all parent bindings into a giant `Map<Key, ResolvedBindings>`. Instead, the resolver can first look up in its own `resolvedContributionBindings` map and just delegate to its parent if the key doesn't exist.

This can save quite a bit of resources for cases where a project has many small subcomponents branched from a larger component since each small subcomponent doesn't have to copy over all of the bindings from the larger component into its own `Map<Key, ResolvedBindings>`.

Note: This is a similar optimization to CL/578306878.
RELNOTES=N/A
PiperOrigin-RevId: 583124976
  • Loading branch information
bcorso authored and Dagger Team committed Nov 16, 2023
1 parent f0c2510 commit 85e9ff1
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.common.graph.MutableNetwork;
import com.google.common.graph.NetworkBuilder;
import dagger.internal.codegen.binding.BindingGraph.TopLevelBindingGraph;
import dagger.internal.codegen.binding.BindingGraphFactory.LegacyBindingGraph;
import dagger.internal.codegen.binding.ComponentDescriptor.ComponentMethodDescriptor;
import dagger.internal.codegen.model.BindingGraph.ComponentNode;
import dagger.internal.codegen.model.BindingGraph.DependencyEdge;
Expand Down
97 changes: 75 additions & 22 deletions java/dagger/internal/codegen/binding/BindingGraphFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import androidx.room.compiler.processing.XTypeElement;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
Expand All @@ -51,11 +50,13 @@
import dagger.internal.codegen.base.OptionalType;
import dagger.internal.codegen.compileroption.CompilerOptions;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.model.BindingGraph.ComponentNode;
import dagger.internal.codegen.model.BindingKind;
import dagger.internal.codegen.model.ComponentPath;
import dagger.internal.codegen.model.DaggerTypeElement;
import dagger.internal.codegen.model.DependencyRequest;
import dagger.internal.codegen.model.Key;
import dagger.internal.codegen.model.RequestKind;
import dagger.internal.codegen.model.Scope;
import dagger.internal.codegen.xprocessing.XTypeElements;
import java.util.ArrayDeque;
Expand Down Expand Up @@ -244,12 +245,7 @@ private LegacyBindingGraph createLegacyBindingGraph(
}
}

return new LegacyBindingGraph(
componentPath,
componentDescriptor,
ImmutableMap.copyOf(requestResolver.getResolvedContributionBindings()),
ImmutableMap.copyOf(requestResolver.getResolvedMembersInjectionBindings()),
ImmutableList.copyOf(subgraphs.build()));
return new LegacyBindingGraph(requestResolver, subgraphs.build());
}

/**
Expand Down Expand Up @@ -308,6 +304,68 @@ public void clearCache() {
keysMatchingRequestCache.clear();
}

/** Represents a fully resolved binding graph. */
static final class LegacyBindingGraph {
private final Resolver resolver;
private final ImmutableList<LegacyBindingGraph> resolvedSubgraphs;
private final ComponentNode componentNode;

LegacyBindingGraph(Resolver resolver, ImmutableList<LegacyBindingGraph> resolvedSubgraphs) {
this.resolver = resolver;
this.resolvedSubgraphs = resolvedSubgraphs;
this.componentNode =
ComponentNodeImpl.create(resolver.componentPath, resolver.componentDescriptor);
}

/** Returns the {@link ComponentNode} associated with this binding graph. */
ComponentNode componentNode() {
return componentNode;
}

/** Returns the {@link ComponentPath} associated with this binding graph. */
ComponentPath componentPath() {
return resolver.componentPath;
}

/** Returns the {@link ComponentDescriptor} associated with this binding graph. */
ComponentDescriptor componentDescriptor() {
return resolver.componentDescriptor;
}

/**
* Returns the {@link ResolvedBindings} in this graph or a parent graph that matches the given
* request.
*
* <p>An exception is thrown if there are no resolved bindings found for the request; however,
* this should never happen since all dependencies should have been resolved at this point.
*/
ResolvedBindings resolvedBindings(BindingRequest request) {
return request.isRequestKind(RequestKind.MEMBERS_INJECTION)
? resolver.getResolvedMembersInjectionBindings(request.key())
: resolver.getResolvedContributionBindings(request.key());
}

/**
* Returns all {@link ResolvedBindings} for the given request.
*
* <p>Note that this only returns the bindings resolved in this component. Bindings resolved in
* parent components are not included.
*/
Iterable<ResolvedBindings> resolvedBindings() {
// Don't return an immutable collection - this is only ever used for looping over all bindings
// in the graph. Copying is wasteful, especially if is a hashing collection, since the values
// should all, by definition, be distinct.
return Iterables.concat(
resolver.resolvedMembersInjectionBindings.values(),
resolver.resolvedContributionBindings.values());
}

/** Returns the resolved subgraphs. */
ImmutableList<LegacyBindingGraph> subgraphs() {
return resolvedSubgraphs;
}
}

private final class Resolver {
final ComponentPath componentPath;
final Optional<Resolver> parentResolver;
Expand Down Expand Up @@ -849,23 +907,18 @@ private void resolveDependencies(ResolvedBindings resolvedBindings) {
}
}

/**
* Returns all of the {@link ResolvedBindings} for {@link ContributionBinding}s from this and
* all ancestor resolvers, indexed by {@link ResolvedBindings#key()}.
*/
Map<Key, ResolvedBindings> getResolvedContributionBindings() {
Map<Key, ResolvedBindings> bindings = new LinkedHashMap<>();
parentResolver.ifPresent(parent -> bindings.putAll(parent.getResolvedContributionBindings()));
bindings.putAll(resolvedContributionBindings);
return bindings;
private ResolvedBindings getResolvedContributionBindings(Key key) {
if (resolvedContributionBindings.containsKey(key)) {
return resolvedContributionBindings.get(key);
}
if (parentResolver.isPresent()) {
return parentResolver.get().getResolvedContributionBindings(key);
}
throw new AssertionError("No resolved bindings for key: " + key);
}

/**
* Returns all of the {@link ResolvedBindings} for {@link MembersInjectionBinding} from this
* resolvers, indexed by {@link ResolvedBindings#key()}.
*/
ImmutableMap<Key, ResolvedBindings> getResolvedMembersInjectionBindings() {
return ImmutableMap.copyOf(resolvedMembersInjectionBindings);
private ResolvedBindings getResolvedMembersInjectionBindings(Key key) {
return resolvedMembersInjectionBindings.get(key);
}

private final class LocalDependencyChecker {
Expand Down
94 changes: 0 additions & 94 deletions java/dagger/internal/codegen/binding/LegacyBindingGraph.java

This file was deleted.

0 comments on commit 85e9ff1

Please sign in to comment.