From 82a03d631465d6d20bb7bb4fdb03bc19fa1678d1 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 29 May 2019 23:06:11 +0200 Subject: Make providers lazy --- .../yahoo/container/di/CloudSubscriberFactory.java | 8 +++-- .../java/com/yahoo/container/di/Container.java | 8 ++--- .../di/componentgraph/core/ComponentGraph.java | 9 +++--- .../di/componentgraph/core/ComponentNode.java | 2 +- .../componentgraph/core/ComponentRegistryNode.java | 4 ++- .../di/componentgraph/core/JerseyNode.java | 2 +- .../container/di/componentgraph/core/Node.java | 35 +++++++++++----------- 7 files changed, 36 insertions(+), 32 deletions(-) (limited to 'container-di') diff --git a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index 3f3991760e3..0851f69a366 100644 --- a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -26,6 +26,7 @@ import java.util.logging.Logger; * @author ollivir */ public class CloudSubscriberFactory implements SubscriberFactory { + private static final Logger log = Logger.getLogger(CloudSubscriberFactory.class.getName()); private final ConfigSource configSource; @@ -46,7 +47,7 @@ public class CloudSubscriberFactory implements SubscriberFactory { } CloudSubscriber subscriber = new CloudSubscriber(subscriptionKeys, configSource); - testGeneration.ifPresent(subscriber.subscriber::reload); //TODO: test specific code, remove + testGeneration.ifPresent(subscriber.subscriber::reload); // TODO: test specific code, remove activeSubscribers.put(subscriber, 0); return subscriber; @@ -112,7 +113,7 @@ public class CloudSubscriberFactory implements SubscriberFactory { * component is removed, so this old config generation will soon be replaced by a new one. */ boolean gotNextGen = false; int numExceptions = 0; - while (!gotNextGen) { + while ( ! gotNextGen) { try { if (subscriber.nextGeneration()) { gotNextGen = true; @@ -136,8 +137,8 @@ public class CloudSubscriberFactory implements SubscriberFactory { public void close() { subscriber.close(); } - } + } public static class Provider implements com.google.inject.Provider { @Override @@ -145,4 +146,5 @@ public class CloudSubscriberFactory implements SubscriberFactory { return new CloudSubscriberFactory(ConfigSourceSet.createDefault()); } } + } diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java index 50e87cd9db6..24e5dcef838 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Container.java +++ b/container-di/src/main/java/com/yahoo/container/di/Container.java @@ -67,8 +67,8 @@ public class Container { private void deconstructObsoleteComponents(ComponentGraph oldGraph, ComponentGraph newGraph) { IdentityHashMap oldComponents = new IdentityHashMap<>(); - oldGraph.allComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); - newGraph.allComponentsAndProviders().forEach(oldComponents::remove); + oldGraph.allConstructedComponentsAndProviders().forEach(c -> oldComponents.put(c, null)); + newGraph.allConstructedComponentsAndProviders().forEach(oldComponents::remove); oldComponents.keySet().forEach(componentDeconstructor::deconstruct); } @@ -226,7 +226,7 @@ public class Container { } private void constructComponents(ComponentGraph graph) { - graph.nodes().forEach(Node::newOrCachedInstance); + graph.nodes().forEach(Node::constructInstance); } public void shutdown(ComponentGraph graph, ComponentDeconstructor deconstructor) { @@ -246,7 +246,7 @@ public class Container { } private void deconstructAllComponents(ComponentGraph graph, ComponentDeconstructor deconstructor) { - graph.allComponentsAndProviders().forEach(deconstructor::deconstruct); + graph.allConstructedComponentsAndProviders().forEach(deconstructor::deconstruct); } public static T getConfig(ConfigKey key, diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java index 76ca94c9286..4a0598d3436 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentGraph.java @@ -43,6 +43,7 @@ import static com.yahoo.container.di.componentgraph.core.Exceptions.removeStackT */ @NotThreadSafe public class ComponentGraph { + private static final Logger log = Logger.getLogger(ComponentGraph.class.getName()); private long generation; @@ -107,7 +108,7 @@ public class ComponentGraph { @SuppressWarnings("unchecked") public T getInstance(Key key) { // TODO: Combine exception handling with lookupGlobalComponent. - Object ob = lookupGlobalComponent(key).map(Node::newOrCachedInstance) + Object ob = lookupGlobalComponent(key).map(Node::component) .orElseThrow(() -> new IllegalStateException(String.format("No global component with key '%s' ", key))); return (T) ob; } @@ -162,8 +163,8 @@ public class ComponentGraph { } } - public Collection allComponentsAndProviders() { - return nodes().stream().map(node -> node.instance().get()).collect(Collectors.toList()); + public Collection allConstructedComponentsAndProviders() { + return nodes().stream().map(node -> node.constructedInstance().get()).collect(Collectors.toList()); } private void completeComponentRegistryNode(ComponentRegistryNode registry) { @@ -243,7 +244,7 @@ public class ComponentGraph { } private Optional matchingGuiceNode(Key key, Object instance) { - return matchingNodes(nodes(), GuiceNode.class, key).stream().filter(node -> node.newOrCachedInstance() == instance). // TODO: assert that there is only one (after filter) + return matchingNodes(nodes(), GuiceNode.class, key).stream().filter(node -> node.component() == instance). // TODO: assert that there is only one (after filter) findFirst(); } diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java index dbe8ded550b..911cd5ffd02 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentNode.java @@ -138,7 +138,7 @@ public class ComponentNode extends Node { List actualArguments = new ArrayList<>(); for (Object ob : arguments) { if (ob instanceof Node) { - actualArguments.add(((Node) ob).newOrCachedInstance()); + actualArguments.add(((Node) ob).component()); } else if (ob instanceof ConfigKey) { actualArguments.add(availableConfigs.get(ob)); } else { diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentRegistryNode.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentRegistryNode.java index 8af1713c84f..429052c0039 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentRegistryNode.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/ComponentRegistryNode.java @@ -19,6 +19,7 @@ import java.util.stream.Collectors; * @author ollivir */ public class ComponentRegistryNode extends Node { + private static ComponentId componentRegistryNamespace = ComponentId.fromString("ComponentRegistry"); private final Class componentClass; @@ -36,7 +37,7 @@ public class ComponentRegistryNode extends Node { @Override protected Object newInstance() { ComponentRegistry registry = new ComponentRegistry<>(); - componentsToInject.forEach(component -> registry.register(component.componentId(), component.newOrCachedInstance())); + componentsToInject.forEach(component -> registry.register(component.componentId(), component.component())); return registry; } @@ -102,4 +103,5 @@ public class ComponentRegistryNode extends Node { return false; } } + } diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/JerseyNode.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/JerseyNode.java index 79b849bff8f..0f8aa678934 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/JerseyNode.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/JerseyNode.java @@ -47,7 +47,7 @@ public class JerseyNode extends ComponentNode { } componentsToInject.forEach(component -> restApiContext.addInjectableComponent(component.instanceKey(), component.componentId(), - component.newOrCachedInstance())); + component.component())); return restApiContext; } diff --git a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/Node.java b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/Node.java index 6feac7a4078..9ed89a6091a 100644 --- a/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/Node.java +++ b/container-di/src/main/java/com/yahoo/container/di/componentgraph/core/Node.java @@ -24,6 +24,7 @@ import static com.yahoo.log.LogLevel.SPAM; * @author ollivir */ public abstract class Node { + private final static Logger log = Logger.getLogger(Node.class.getName()); private final ComponentId componentId; @@ -43,25 +44,23 @@ public abstract class Node { protected abstract Object newInstance(); - public Object newOrCachedInstance() { - Object inst; - if (instance.isPresent()) { - inst = instance.get(); - log.log(SPAM, "Reusing instance for component with ID " + componentId); - } else { - log.log(DEBUG, "Creating new instance for component with ID " + componentId); - inst = newInstance(); - instance = Optional.of(inst); - } - return component(inst); + /** Constructs the instance represented by this node, if not already done. */ + public void constructInstance() { + if ( ! instance.isPresent()) + instance = Optional.of(newInstance()); } - private Object component(Object instance) { - if (instance instanceof Provider) { - Provider provider = (Provider) instance; + /** + * Returns the component represented by this - which is either the instance, or if the instance is a provider, + * the component returned by it. + */ + public Object component() { + constructInstance(); + if (instance.get() instanceof Provider) { + Provider provider = (Provider) instance.get(); return provider.get(); } else { - return instance; + return instance.get(); } } @@ -139,13 +138,13 @@ public abstract class Node { return componentId; } - public Optional instance() { + /** Returns the already constructed instance in this, if any */ + public Optional constructedInstance() { return instance; } /** - * @param identityObject - * The identifying object that makes the Node unique + * @param identityObject he identifying object that makes the Node unique */ protected static ComponentId syntheticComponentId(String className, Object identityObject, ComponentId namespace) { String name = className + "_" + System.identityHashCode(identityObject); -- cgit v1.2.3