My favorites | Sign in
Google
                
New issue | Search
for
| Advanced search | Search tips
Issue 38: Eager loading for any scope
4 people starred this issue and may be notified of changes. Back to list
Status:  Accepted
Owner:  limpbizkit
Type-Enhancement
Priority-Medium


Sign in to add a comment
 
Reported by kevinb9n, Feb 27, 2007
Support eager loading generically for any scope.  Rename
eagerlyInContainer() to just eagerly().
Comment 1 by kevinb9n, Mar 01, 2007
for the 2 scopes besides singleton that we bundle, I think eagerly() doesn't seem to
make a whole lot of sense.  for people's custom scopes they may want it, but I'm
lowering the priority.
Labels: -Priority-Medium Priority-Low
Comment 2 by erik.put...@nrc-cnrc.gc.ca, Dec 15, 2008
Any hopes to have this for the 2.0 release? Custom scopes are nice but not being able
to eagerly load instances is a great limitation.
I don't mind writing a patch if there are any hopes to make it happen.
Comment 3 by crazyboblee, Dec 15, 2008
Erik, do you have a use case you can share?
Comment 4 by sberlin, Dec 15, 2008
I would personally like this so I don't have to abuse the @Singleton stage for things
I want injected as part of a lifecycle.  I can annotate them with @Service or
@Lifecycle, make sure that only that annotation is eagerly loaded, and avoid having
to eagerly load all singletons (which slows startup time -- something that's a
definite issue on client apps).

The workaround I was going to do (after we release LimeWire 5 final) was to change to
a new singleton-like scope & have the lifecycle manager use Guice's SPI to iterate
and find those bindings and eagerly load them.
Comment 5 by crazyboblee, Dec 15, 2008
Sam,

Would having code like this in your modules be acceptable?

  install(service(MyService.class));

If so, you could implement something like this:

  /**
   * Installs a service given its type.
   */
  static <T> Module install(Class<T extends Service> serviceType) {
    return new Module() {
      public void configure(Binder binder) {
        final Provider<T> serviceProvider = binder.getProvider(serviceType);
        binder.requestInjection(new Object() {
          @Inject
          void registerService(ServiceRegistry registry) {
            registry.register(serviceProvider);
          }
        });
      }  
    };
  }

Then, you'd have something like this:

  interface ServiceRegistry {
    void register(Provider<? extends Service> serviceProvider);
    void start();
    void stop();
  }

You could just as easily support:

  install(services(
    ServiceA.class,
    ServiceB.class,
    ...
  ));

Comment 6 by sberlin, Dec 15, 2008
I'm hesitant to bind the services in the Modules -- the reason being that the
registering of the Service I see as an Impl detail, and that the Impls sometimes want
to internally register multiple services.  We have almost exactly that
ServiceRegistry interface:  

public interface ServiceRegistry {
    StagedRegisterBuilder register(Service service);
    void initialize();
    void start();
    void stop();
    void start(Object stage);
    void addListener(ServiceRegistryListener serviceRegistryListener);
}

public interface StagedRegisterBuilder {    
    public void in(ServiceStage stage);    
    public void in(Object customStage);
}

Impls have an

@Inject void register(ServiceRegistry registry) {
   registry.register(this);
}

or

@Inject void register(ServiceRegistry registry) {
  registry.register(new Service() { ... } ).in(StageOne); // one service
  registry.register(new Service() { ... } ).in(StageTwo); // another service
}


Comment 7 by crazyboblee, Dec 15, 2008
Any problem in computer science can be solved with another layer of indirection:

  install(serviceManifest(MyServiceManifest.class));

  interface ServiceManifest {
    void initialize(ServiceRegistry registry);
  }

  static Module serviceManifest(
      Class<? extends ServiceManifest> manifestType) {
    return new Module() {
      public void configure(Binder binder) {
        final Provider<? extends ServiceManifest> manifestProvider 
            = binder.getProvider(manifestType);
        binder.requestInjection(new Object() {
          @Inject
          void registerService(ServiceRegistry registry) {
            ServiceManifest manifest = provider.get();
            manifest.initialize(registry);
          }
        });
      }  
    };
  }

Your service manifest implementation will just have the actual services (or their
providers) injected. That way, you won't be directly instantiating services w/ new.





Comment 8 by sberlin, Dec 15, 2008
Yup, this approach will definitely work.  I just don't like it. :-)

It seems to me that it requires knowledge of the class (interface or impl) being a
service in two places plus it requires binding additional manifest classes.  I'd much
rather say bind(Foo.class).to(FooImpl.class) and FooImpl can decide on its own if
it's a service.  Requiring the additional install(serviceFor(FooImpl.class)) seems
fragile and code-smellish.

If Guice could eagerly bind arbitrary annotations, I could annotate FooImpl with
@Service to it guarantees its @Inject methods get called at constructor time and it'd
automagically work (but not in that "what the hell's going on" kind of way). 
Alternately, the eagerness doesn't really have to happen during injection time -- it
can happen at anytime, by some kind of injector.loadScope(Service.class).

In some ways, the eagerness of a scope is bound to the Stage.  I could envision
allowing arbitrary stages, and Guice asking the Stage impl, "What scope should I
eagerly load?".  If Stage.PRODUCTION could be refactored so that it supplied
@Singleton instead of Guice internally tying the two together, it would open the
doors for other Stages.
Comment 9 by crazyboblee, Dec 15, 2008
Ideally, you'd be able to do all the binding inside of the static method, so you
wouldn't need to repeat the same information in two places. I don't have enough
detail about how you set up services though.

Guice doesn't support eager initialization w/ annotations because there's no
guarantee that Guice will know about that type at startup (in which case it won't see
the annotation). That's why we don't support @Singleton(eager=true). I'm morally
opposed to classpath scanning.

While what you're proposing will solve your problem, I don't think it will carry its
weight for everyone else, so it's much better if we can come up with an adequate
solution using the current APIs.

If you really do want everything to be triggered from a @Service annotation, are you
familiar with JSR 269? You could write an annotation processor that generates a Guice
module based on all of the classes it sees annotated w/ @Service. The generated class
could also be useful from a debugging standpoint.
Comment 10 by sberlin, Dec 16, 2008
I'm not sure I follow on how you wouldn't need to repeat the info in two places.  If
I have an interface FooManager, I might have an impl FoomManagerImpl that is simple &
keeps its state locally, or an impl SmartFooManagerImpl that requires loading a
background thread and occasionally doing some work (maybe by dumping old data).  The
loading a background thread is what makes SmartFooManagerImpl a Service requiring a
lifecycle -- at some point it needs to start, and at another point it needs to stop.

It seems that if there's any requirement I write code in the Module to call
SmartFooManagerImpl a "service", then I'm requiring code in two places.  One in the
Impl itself to make it a service, and another in the module to bind it as such.

Consider someone refactoring FooManagerImpl and deciding it needs to be a service. 
One approach is to keep it localized and use @Inject void register(ServiceRegistry
registry) within FooManagerImpl [and somehow make sure it's eagerly loaded].  The
other is to expose the Service information and bind it as such in the module.  IMO,
the latter approach is more complicated and prone to errors as people continue to
refactor & add/remove services.

Re: @Singleton(eager=true) -- I follow the logic, but I don't reach the same
conclusion.  You don't need to allow classpath scanning.  Just a FAQ entry that says,
"The singleton must be bound or required by another bound class for Guice to load
it."  A simple bind(MySingleton.class); in the module will do the trick.  I'd much
rather keep the knowledge that it's a service (and hence needs eager loading) within
the impl itself instead of splitting that knowledge (the code that makes it a service
vs the binding that makes it eager) between the impl & the module.
Comment 12 by erik.put...@nrc-cnrc.gc.ca, Jan 29, 2009
Somehow I did not get a notification about this issue... 

Here is my test case, I have an ApplicationScope which processes @PreDestroy and 
@PostConstruct annotations. For instance, I start the database with this application 
scope. Then, I have an embedded web service which needs the database. This class is
not referenced anywhere else so there won't be any lazy creation happening. And
without lazy creation I'm not sure how I can register one instance to my
ApplicationScope. And I need a way to start the web service after the database is
started. I was hoping that a asEager(Scope scope) could solve my problem.
Comment 13 by erik.put...@nrc-cnrc.gc.ca, Jan 29, 2009
I found a workaround... If I have a class A which needs to be instantiated in a
scope, I create a class B which injects class A in the constructor and I request
bind(B.class).asEagerSingleton()
Comment 14 by sberlin, Jan 29, 2009
I would think that breaks scoping.  I haven't used scopes much, but it seems like a
"smaller" scope should be able to inject a "larger" scope, but a larger one shouldn't
take a smaller one.  That is, an @SessionScope object can inject an @Singleton one,
but an @Singleton object cannot take an @SessionScope one.  Otherwise, there wouldn't
be any valid scope for the Singleton to use.
Comment 15 by erik.putrycz, Jan 29, 2009
This is definitely just a workaround. I'd rather write bind(A.class).asEager
(StarteableScope.SCOPE). Also I don't believe that scopes need a hierarchy. My 
custom scope is for "starteable" objects, they can be singletons or not. 
Comment 16 by sberlin, Jun 05, 2009
I spoke to Jesse a little about this today, but figure I might as well expand upon it
and put my thoughts onto the bug.  The background:  We have a lot of singletons that
need to be singletons.  We can't load them all eagerly, because it destroys startup
time (due to excessive classloading, object creation, etc..).  There's a select few
that we want to load eagerly.  I've tried getting developers to use a custom
@LazySingleton annotation for singletons that can be lazy (and using Stage.PRODUCTION
to eagerly load @Singletons), but it fails miserably.

The problems with that approach are: 1) If the lazy singleton is injected into any
other singleton, it promotes itself to an eager singleton.  This can be solved by
always using a Provider<...>, but that's incredibly annoying and very error-prone. 
2) People in general think "Oh, this should be a singleton.  I'll add @Singleton!" 
People don't think, "This should be a singleton, I'll add @LazySingleton!"  It's just
not the flow people go through.

We want lazy to be the default and eager to be the exception. This is WITHOUT
requiring that the Module explicitly has bind(...).asEagerSingleton(), because it's
better to see the implicit documentation in the class itself (@EagerSingleton), and
it's good to be able to not eagerly load it for tests.

So, to fix this, I created an @EagerSingleton annotation and tried to get Guice to
auto-eager these bindings.  (I purposely am avoiding using Module rewriting looking
for changes because I don't want to force users of the Scope to use a module-rewrite
whenever a module binds to this scope.  Also, I want it to work on JIT bindings.) 
For the most part, it works magically.  The major problem is that it doesn't work on
JIT bindings.  This manifests itself in two ways.  If the JIT binding is discovered
during Injector creation, the binding is just lost into the ether.  If the JIT
binding is discovered after Injector creation (eg, Injector.getInstance(aNewClass)),
it blows up.

Attached is the Module that makes it happen, the Scope it uses & a test for the
behavior I'd like (the explicit binding tests pass, the JIT ones fail).

There are two approaches I can see to fixing the failures.  Both may be necessary.

1) Expose JIT bindings from the Injector.  Injector.getBindings only return explicit
bindings.  Perhaps a new method, Injector.getJustInTimeBindings, so that literally
every binding can be introspected.

2) Introduce a way to listen to bindings once they're created and ready for use.  The
Binder.bindListener method is excellent for listening to classes & injected objects
and doing funky things for custom injection.  But it's not the best way for listening
to bindings and doing funky things before the binding is realized into an object. 
Something like, Binder.bindBindingListener(Matcher<? super Binding<?>>,
BindingListener), where BindingListener has 'hear(Binding<I>)'.  (It could have a
BindEncounter too, but I'm not sure of the need for it.)

Of course, there's very likely a whole different way of solving this issue... but I
do see these two problem-points as general-purpose problems that would expose a lot
of functionality if solved.


LimeWireInjectModule.java
2.4 KB   Download
MoreScopes.java
1.0 KB   Download
EagerSingleton.java
695 bytes   Download
EagerSingletonTest.java
4.1 KB   Download
Comment 17 by sberlin, Jun 05, 2009
As an addendum the prior note, the strategy also fails when binding an interface to
an implementation (where the implementation is bound implicitly through a JIT binding).

If I had to pick for just one of the above two suggestions, I'd go with #1, as it
would at least allow me to simulate the current eager singleton loading.  As it
stands now, it's impossible without hacking at the internals of Guice.
Comment 18 by limpbizkit, Jun 06, 2009
Adding an API to lookup JIT bindings is a great idea. There's two different approaches we could take for the 
implementation:
  A) return a "live" map, so that JIT bindings appear within as they're formed
  B) return a snapshot of the JIT bindings at the time of the request
Because we don't have built-in observable maps, there's not tooo much utility to going with A. If we went the 
live route, we'd still want to add a callback API so that the observer could discover when a binding is formed. 

Therefore I think we should just do B. The method would lock on the jitBindings map, copy them (into say, 
an ImmutableMap), and return the copy. 

This leaves only two potential problems: what to name the method, and whether it should contain ALL 
bindings, or just the just-in-time ones. Either is sufficient, but I think I prefer to include only the JIT ones, 
since otherwise the "getBindings" method seems misnamed. Some candidates for the name: "getJustInTimeBindings", "getSyntheticBindings", and "getInferredBindings".

Sam - would you mind writing the code (and a test) ?

Owner: limpbizkit
Labels: -Priority-Low Priority-Medium
Comment 19 by alen_vre...@yahoo.com, Jun 06, 2009
A couple of thoughts. How about having @Lazy and @Eager annotations? 

@Singleton tells me precisely what it does - that this object will live forever. But
when is the object born(eager/lazy) is something I don't consider to be in the
mindset of longevity(scopes). Just like you mention it's just
not the flow people go through.

(@MyCustomScope|@Singleton|...) (@Eager|@Lazy)
public class Foo{}

Wanted to try out the listeners for some time. I made a listener that collects all
the eager types it hears and after the injector is created they get eagerly created.
JIT bindings should work except the toProvider problem.
EagerAbstractModule.java
4.4 KB   Download
Comment 20 by sberlin, Jun 06, 2009
Jesse:  Attached is a patch + tests for Injector.getJustInTimeBindings.  I'll hack
away at a listener for new bindings on the flight back to NY.

Alen: I view lazy & eager as mutations of the singleton state.  A lazy/eager
non-singleton, or a lazy/eager session-scoped object doesn't make much sense. 
Because Guice allows different ways for scopes to be set (by explicit binding or by
annotation), looking at the class' annotations may be a misleading view of what scope
is applied.  (If the binding explicitly specifies a scope, the annotation on the
class is ignored.)  Because of that, on my earlier patch I opted against iterating
over the class' annotations and instead went with making eager & lazy full-fledged
scopes, as it was the only way I could think of to make sure I was dealing with the
proper scope.


Comment 21 by sberlin, Jun 06, 2009
Oops, forgot to attach patch.  Now attached.  (That attach file button should be
further away from 'Save changes' :) ).
exposedJitBindings.txt
6.9 KB   Download
Comment 22 by sberlin, Jun 06, 2009
Attached is a cut at adding binding listeners.  The code mirrors TypeListeners a
whole lot (but is different enough that it can't really share the code).  You bind a
binding listener by using bindBindingListener(..).  Explicit & JIT bindings
discovered while creating explicit bindings are notified just before eager singletons
are created.  JIT bindings discovered after that point are notified as they're
discovered.  There's a new test at BindingListenerTest.  (Lots more test methods
could be added, but figured I would post this patch to get some feedback on the
architecture.)

The patch is based on the code from before the big refactor & also contains the patch
for exposing JIT bindings, sorry.

(This patch delivered to you by American Airline's in-flight wireless internet.) 
bindingListeners.txt
32.7 KB   Download
Comment 23 by sberlin, Jun 06, 2009
... and the same patch as above, but can be applied after the refactoring.  Turned
out to be a lot easier to fix than I had thought (never experienced just how nice
SVN's 'move' functions are).

(Sorry for the influx of patches.)
listeners-after-refactor.txt
31.7 KB   Download
Comment 24 by alen_vre...@yahoo.com, Jun 08, 2009
Last thought. How about generalizing on the preloaders? Right now asEagerSingleton()
is a special case of adding a preloader when stage is not production and the scope is
singleton.

Instead of asEagerSingleton() we could have preloaded() or eagerly() or something
that works with arbitrary scope like already mentioned. Further more a scope could be
declared preloadable e.g. your EagerSingleton scope, and JIT could be preloadable (in
case of toProvider call). Sam you're right this(eager/preloading) could be viewed as
a mutation of the scope.
Comment 25 by sberlin, Jun 08, 2009
One nifty thing about this 'binding listener' patch is that it lets you easily
generalize preloaders.  The existing built-in code (and logic) for preloading eager
singletons can be rewritten as a listener like:

  bindBindingListener(new AbstractMatcher<Binding>() {
     public boolean matches(Binding b) {
        return bindingIsEagerSingletonorSingletonAndStageIsProduction();
     }, new BindingListener() {
        public void hear(Binding b) {
           b.getProvider().get();
        }
     });

And users can supply any of their custom binding listeners to do whatever preloading
they'd like.
Comment 26 by sberlin, Jun 08, 2009
...and it also fixes the case Jesse posted to the dev list the other day, where you have:

  @Singleton static class Foo {}
  Injector i = Guice.createInjector(Stage.PRODUCTION);
  Provider<Foo> i.getProvider(Foo.class);
  // Foo is now preloaded (constructed), even though it was discovered
  // after the injector finished loading.
Comment 27 by alen_vre...@yahoo.com, Jun 08, 2009
At first I thought this is something I want to see in core Guice but some things are
better left to extensions. Thanks for making this clear to me.

Gave the patch a go. Nice work. I got the preloading working, as I wanted, in no
time. I do find it strange that using a PrivateModule with expose the exposed binding
will get heard 2 times.
Comment 28 by sberlin, Jun 08, 2009
That does seem strange.  Not sure what the proper behavior should be, but it sounds
like it'd be a bug.  I don't use private or child modules much, so am not very
familiar with them.  I'll wait to see whether the patch has a chance of being
integrated before doing more work on it.
Comment 29 by sberlin, Jun 11, 2009
moved patch to separate issue @ issue 387 (because it solves other issues than just
this one)
Sign in to add a comment