mrszj / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Eager loading for any scope #38

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Support eager loading generically for any scope.  Rename
eagerlyInContainer() to just eagerly().

Original issue reported on code.google.com by kevin...@gmail.com on 27 Feb 2007 at 8:28

GoogleCodeExporter commented 9 years ago
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.

Original comment by kevin...@gmail.com on 1 Mar 2007 at 8:09

GoogleCodeExporter commented 9 years ago
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.

Original comment by erik.put...@nrc-cnrc.gc.ca on 15 Dec 2008 at 8:28

GoogleCodeExporter commented 9 years ago
Erik, do you have a use case you can share?

Original comment by crazybob...@gmail.com on 15 Dec 2008 at 8:54

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 15 Dec 2008 at 9:17

GoogleCodeExporter commented 9 years ago
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,
    ...
  ));

Original comment by crazybob...@gmail.com on 15 Dec 2008 at 10:42

GoogleCodeExporter commented 9 years ago
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
}

Original comment by sberlin on 15 Dec 2008 at 11:06

GoogleCodeExporter commented 9 years ago
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.

Original comment by crazybob...@gmail.com on 15 Dec 2008 at 11:28

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 15 Dec 2008 at 11:40

GoogleCodeExporter commented 9 years ago
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.

Original comment by crazybob...@gmail.com on 15 Dec 2008 at 11:56

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 16 Dec 2008 at 9:59

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
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.

Original comment by erik.put...@nrc-cnrc.gc.ca on 29 Jan 2009 at 4:31

GoogleCodeExporter commented 9 years ago
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()

Original comment by erik.put...@nrc-cnrc.gc.ca on 29 Jan 2009 at 7:42

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 29 Jan 2009 at 7:45

GoogleCodeExporter commented 9 years ago
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. 

Original comment by erik.put...@gmail.com on 29 Jan 2009 at 7:55

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 6 Jun 2009 at 5:05

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 6 Jun 2009 at 5:27

GoogleCodeExporter commented 9 years ago
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) ?

Original comment by limpbizkit on 6 Jun 2009 at 8:10

GoogleCodeExporter commented 9 years ago
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.

Original comment by alen_vre...@yahoo.com on 6 Jun 2009 at 1:05

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 6 Jun 2009 at 5:21

GoogleCodeExporter commented 9 years ago
Oops, forgot to attach patch.  Now attached.  (That attach file button should be
further away from 'Save changes' :) ).

Original comment by sberlin on 6 Jun 2009 at 5:22

Attachments:

GoogleCodeExporter commented 9 years ago
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.) 

Original comment by sberlin on 6 Jun 2009 at 10:53

Attachments:

GoogleCodeExporter commented 9 years ago
... 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.)

Original comment by sberlin on 6 Jun 2009 at 11:07

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by alen_vre...@yahoo.com on 8 Jun 2009 at 10:07

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 8 Jun 2009 at 2:15

GoogleCodeExporter commented 9 years ago
...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.

Original comment by sberlin on 8 Jun 2009 at 2:18

GoogleCodeExporter commented 9 years ago
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.

Original comment by alen_vre...@yahoo.com on 8 Jun 2009 at 7:58

GoogleCodeExporter commented 9 years ago
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.

Original comment by sberlin on 8 Jun 2009 at 9:03

GoogleCodeExporter commented 9 years ago
moved patch to separate issue @ issue 387 (because it solves other issues than 
just
this one)

Original comment by sberlin on 12 Jun 2009 at 3:52

GoogleCodeExporter commented 9 years ago

Original comment by christia...@gmail.com on 4 Jun 2012 at 5:45

GoogleCodeExporter commented 9 years ago
A solution for this can be worked-around using the SPI, and the project is 
unlikely to incorporate this directly.  

Original comment by cgruber@google.com on 18 Nov 2013 at 9:04