From 8d9c84f84e0e7ddd56366cb640d6670b2989d014 Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Fri, 5 Jul 2013 17:29:35 -0700 Subject: [PATCH] optimize guice injector once created in guice, we always use eager loaded singletons for all modules we create, thus, we can actually optimize the memory used by injectors by reduced the construction information they store per binding resulting in extensive reduction in memory usage for many indices/shards case on a node also because all are eager singletons (and effectively, read only), we can not go through trying to create just in time bindings in the parent injector before trying to craete it in the current injector, resulting in improvement of object creations time and the time it takes to create an index or a shard on a node --- .../common/inject/InheritingState.java | 19 +++++++++-- .../common/inject/InjectorImpl.java | 32 +++++++++++++++++-- .../common/inject/ModulesBuilder.java | 6 ++++ .../elasticsearch/common/inject/State.java | 6 ++++ .../inject/internal/InternalFactory.java | 23 +++++++++++++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/inject/InheritingState.java b/src/main/java/org/elasticsearch/common/inject/InheritingState.java index 5747e537e04..71d0d82abce 100644 --- a/src/main/java/org/elasticsearch/common/inject/InheritingState.java +++ b/src/main/java/org/elasticsearch/common/inject/InheritingState.java @@ -16,11 +16,11 @@ package org.elasticsearch.common.inject; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import org.elasticsearch.common.inject.internal.BindingImpl; -import org.elasticsearch.common.inject.internal.Errors; -import org.elasticsearch.common.inject.internal.MatcherAndConverter; +import org.elasticsearch.common.inject.internal.*; +import org.elasticsearch.common.inject.spi.InjectionPoint; import org.elasticsearch.common.inject.spi.TypeListenerBinding; import java.lang.annotation.Annotation; @@ -131,6 +131,19 @@ class InheritingState implements State { blacklistedKeys = new WeakKeySet(); } + @Override + public void makeAllBindingsToEagerSingletons(Injector injector) { + Map, Binding> x = Maps.newLinkedHashMap(); + for (Map.Entry, Binding> entry : this.explicitBindingsMutable.entrySet()) { + Key key = entry.getKey(); + BindingImpl binding = (BindingImpl) entry.getValue(); + Object value = binding.getProvider().get(); + x.put(key, new InstanceBindingImpl(injector, key, SourceProvider.UNKNOWN_SOURCE, new InternalFactory.Instance(value), ImmutableSet.of(), value)); + } + this.explicitBindingsMutable.clear(); + this.explicitBindingsMutable.putAll(x); + } + public Object lock() { return lock; } diff --git a/src/main/java/org/elasticsearch/common/inject/InjectorImpl.java b/src/main/java/org/elasticsearch/common/inject/InjectorImpl.java index 79aed0bb416..efafe9ab761 100644 --- a/src/main/java/org/elasticsearch/common/inject/InjectorImpl.java +++ b/src/main/java/org/elasticsearch/common/inject/InjectorImpl.java @@ -42,7 +42,8 @@ import static org.elasticsearch.common.inject.internal.Annotations.findScopeAnno class InjectorImpl implements Injector, Lookups { final State state; final InjectorImpl parent; - final BindingsMultimap bindingsMultimap = new BindingsMultimap(); + boolean readOnly; + BindingsMultimap bindingsMultimap = new BindingsMultimap(); final Initializer initializer; /** @@ -564,7 +565,7 @@ class InjectorImpl implements Injector, Lookups { private BindingImpl createJustInTimeBindingRecursive(Key key, Errors errors) throws ErrorsException { // ask the parent to create the JIT binding - if (parent != null) { + if (parent != null && !parent.readOnly /* ES: don't check on parent if its read only, its already created all the bindings it can*/) { try { return parent.createJustInTimeBindingRecursive(key, new Errors()); } catch (ErrorsException ignored) { @@ -750,8 +751,24 @@ class InjectorImpl implements Injector, Lookups { Provider getProviderOrThrow(final Key key, Errors errors) throws ErrorsException { final InternalFactory factory = getInternalFactory(key, errors); - final Dependency dependency = Dependency.get(key); + // ES: optimize for a common case of read only instance getting from the parent... + if (factory instanceof InternalFactory.Instance) { + return new Provider() { + @Override + public T get() { + try { + return (T) ((InternalFactory.Instance) factory).get(null, null, null); + } catch (ErrorsException e) { + // ignore + } + // should never happen... + assert false; + return null; + } + }; + } + final Dependency dependency = Dependency.get(key); return new Provider() { public T get() { final Errors errors = new Errors(dependency); @@ -833,4 +850,13 @@ class InjectorImpl implements Injector, Lookups { membersInjectorStore = new MembersInjectorStore(this, state.getTypeListenerBindings()); jitBindings = Maps.newHashMap(); } + + // ES_GUICE: make all registered bindings act as eager singletons + public void readOnlyAllSingletons() { + readOnly = true; + state.makeAllBindingsToEagerSingletons(this); + bindingsMultimap = new BindingsMultimap(); + // reindex the bindings + index(); + } } diff --git a/src/main/java/org/elasticsearch/common/inject/ModulesBuilder.java b/src/main/java/org/elasticsearch/common/inject/ModulesBuilder.java index 1a9212ac4a1..7ab0e34b7ec 100644 --- a/src/main/java/org/elasticsearch/common/inject/ModulesBuilder.java +++ b/src/main/java/org/elasticsearch/common/inject/ModulesBuilder.java @@ -58,6 +58,9 @@ public class ModulesBuilder implements Iterable { Modules.processModules(modules); Injector injector = Guice.createInjector(modules); Injectors.cleanCaches(injector); + // in ES, we always create all instances as if they are eager singletons + // this allows for considerable memory savings (no need to store construction info) as well as cycles + ((InjectorImpl) injector).readOnlyAllSingletons(); return injector; } @@ -65,6 +68,9 @@ public class ModulesBuilder implements Iterable { Modules.processModules(modules); Injector childInjector = injector.createChildInjector(modules); Injectors.cleanCaches(childInjector); + // in ES, we always create all instances as if they are eager singletons + // this allows for considerable memory savings (no need to store construction info) as well as cycles + ((InjectorImpl) childInjector).readOnlyAllSingletons(); return childInjector; } } diff --git a/src/main/java/org/elasticsearch/common/inject/State.java b/src/main/java/org/elasticsearch/common/inject/State.java index 208d4f3104b..b9a4f81d3e9 100644 --- a/src/main/java/org/elasticsearch/common/inject/State.java +++ b/src/main/java/org/elasticsearch/common/inject/State.java @@ -92,6 +92,10 @@ interface State { public void clearBlacklisted() { } + @Override + public void makeAllBindingsToEagerSingletons(Injector injector) { + } + public Object lock() { throw new UnsupportedOperationException(); } @@ -156,4 +160,6 @@ interface State { // ES_GUICE: clean blacklist keys void clearBlacklisted(); + + void makeAllBindingsToEagerSingletons(Injector injector); } diff --git a/src/main/java/org/elasticsearch/common/inject/internal/InternalFactory.java b/src/main/java/org/elasticsearch/common/inject/internal/InternalFactory.java index f06ab58d740..c1b36d6a9d4 100644 --- a/src/main/java/org/elasticsearch/common/inject/internal/InternalFactory.java +++ b/src/main/java/org/elasticsearch/common/inject/internal/InternalFactory.java @@ -25,6 +25,29 @@ import org.elasticsearch.common.inject.spi.Dependency; */ public interface InternalFactory { + /** + * ES: + * An factory that returns a pre created instance. + */ + public static class Instance implements InternalFactory { + + private final T object; + + public Instance(T object) { + this.object = object; + } + + @Override + public T get(Errors errors, InternalContext context, Dependency dependency) throws ErrorsException { + return object; + } + + @Override + public String toString() { + return object.toString(); + } + } + /** * Creates an object to be injected. *