Circular Dependencies & Proxies

17 views
Skip to first unread message

Sam Berlin

unread,
Jul 28, 2010, 9:05:17 AM7/28/10
to google-g...@googlegroups.com
I think you are vastly overestimating the amount of times Guice will create a proxy object.  Guice will continue working fine (without proxying) if
  a) there is no circular dependency at all,
  b) dependencies are injected using Provider<X> instead of X
  c) the the circular dependency is only within setters methods (not constructors)
  d) if the circular dependency exists in the setter method of the first constructed object (even if it's in the constructor of the second one).

Of those cases, the only one that could stand to be improved is d).  The improvement would make it so that the proxies aren't constructed even if the reverse situation is true (constructor in first object, setter in second).  But, given that there's other perfectly viable ways to fix it -- such as injecting Provider<X> instead of X -- I'm not as down on the proxies as you.

If you absolutely want to forbid Guice from creating a proxy, because it's ruining your logic, then use disableCircularProxies and tell Guice to generate an exception whenever it was going to create a proxy.  Then look through the code and re-arrange it (either removing the circular dependencies, using Provider, etc..) so that Guice will not create a proxy.

sam

2010/7/28 Endre Stølsvik <stol...@gmail.com>
So the "don't proxy" method is basically worthless, then? It will pretty much always "crash" with "won't proxy" exception for anything but the basicest object graphs, and even sticking in a setter won't resolve it?

How much more time are we talking about here? When the Injector are made, you ONCE set up a tree/graph for how to construct each object, and that would take such a huge time? I can't seem to understand how that would influence Guice that radically to the negative.

Endre.


On Tue, Jul 27, 2010 at 22:55, Sam Berlin <sbe...@gmail.com> wrote:
Yes, this is the way Guice creates objects.  It makes no attempt to derive dependencies and instead constructs objects as requested.  Any attempt to derive an order would have an impact on the speed of Injector creation.

sam

2010/7/27 Endre Stølsvik <stol...@gmail.com>

On Tue, Jul 27, 2010 at 05:06, Dhanji R. Prasanna <dha...@gmail.com> wrote:
(I mean, you can't really inject until a dep is injected first anyway, so this ordering is natural).

I agree that this order is natural - but that is where I have understood that Guice didn't bother: It just injected down the road, supplying proxies wherever an object wasn't yet instantiated. When these objects became instantiated, all the proxies had it set, so that the calls went through. All very transparent, until some constructor (or even setter) tries to invoke something on the injected proxy before the underlying object existed - and you get that "This is a proxy!" exception..

Because of this graph ignorance, you could end up with lots of proxies in your graph, precisely because no such attempt at ordering them in the best way was done.

This understanding comes from reading mailing list throughout the years regarding the proxies. But of course, I might have misunderstood it all! However, if I have understood it anywhere correctly, then the disableCircularProxies method should make injection crash with this new exception much of the time in anything but dead simple graphs.

If Bob is around, he should be able to set this straight in a few sentences, because I believe it is from him I have this understanding, wrong or right..!

Kind regards,
Endre.
 

Dhanji.

2010/7/26 Endre Stølsvik <stol...@gmail.com>

Btw, there have been talk of a dependency graph algorithm thing to go with this feature. Apparently Guice of before just instantiated the objects as it went along, proxying here and there if it didn't have the objects, not distinguishing between constructor and setter/field injecting - thus implementing a setter to "resolve" the fact that you were given a proxy didn't necessarily work out. Guice should rather instantiate after some logic; I guess building some kind of tree of the dependencies, instantiating the leaf nodes (those w/o dependencies) first, then going upwards. Any circle in the constructor graph should resolve correctly by having any of the classes in the circle implement a setter or a field injection taking the object(s) it depends on which is a part of the circle.

Is this how it was implemented?

Thanks again,
Endre.


On Mon, Jul 26, 2010 at 00:56, Sam Berlin <sbe...@gmail.com> wrote:
Yes, it should disable it. I *think* the only time guice created a proxy was when it detected a circular dependency.  If you build the injector with this option, then where guice would have created a proxy, it instead throws an exeption saying circular proxies are disabled. 

Sam


On Jul 24, 2010, at 5:00 AM, Endre Stølsvik <stol...@gmail.com> wrote:

Oh, I didn't realize that it had been implemented!

If this method totally disables Guice's use of proxies (so that it never proxies an object as it did before, you will get the actual objects injected, always), then that is what I've been wanting since day 0! Thanks!

Kind regards,
Endre.

On Fri, Jul 23, 2010 at 14:16, Sam Berlin <sbe...@gmail.com> wrote:
I'm not sure what you mean by "please evaluate the proxies".  Do you mean circular proxies, and you want to disable them?  See InjectorBuilder.disableCircularProxies.

sam

2010/7/23 Endre Stølsvik <stol...@gmail.com>

Please evaluate the proxies - an option to turn that idea off is my personal top priority for Guice. The construction of objects should be done such that they aren't needed, or, if cycles exists, any setter/field injection on any of the classes in question should resolve it.

Endre.

On Sun, Jul 18, 2010 at 01:56, Sam Berlin <sbe...@gmail.com> wrote:
Here's another status update (in an effort to get 3.0 out soon) --

Open issues:

 * 366 - @Provides doesn't respect @Nullable
   Status: Patch attached [from me], needs review (it adds APIs).

 * 436 - Provide access to Guice's own internal TypeConverters
   Status: Patch attached [from Stuart], needs review (it adds APIs, makes some classes public, and changes a bit of the internals).

 * 508 - AOP visibility checks should include the method return type.
   Status: Patch attached [from Stuart], just waiting on some tests before applying.

 * 509 - Cache result of Class.getAnnotations()
   Status: Perf improvement request, no patch attached.

 * 510 -  Remove InjectorBuilder
   Status: Rationale for keeping InjectorBuilder listed in the comments along with other possible scenarios, awaiting replies.

 * 513 - Why do we need Injector.getAllBindings?
   Status: Rationale for why it's useful in the comments, awaiting replies.

 * 514 - Do we need Injector.getExistingBinding()?
   Status: Rationale for why it's useful (but not strictly necessary) in the comments, awaiting replies.

 * 515 - What are the use cases for @Toolable?
   Status: Use cases listed in the comments, awaiting replies.

 * 516 - Investigate duplicate binding filtering
   Status: Performance implications & rationale explained in comments, awaiting replies.

 * 517 - Fix classloader leak
   Status: More questions asked in the comments, awaiting answers.

 * 518 -  Hide Key.ofType()
   Status: Question opened, no replies.  Awaiting comments.

 * 519 - toProvider() should use Guice's Provider, not JSR-330
   Status: Lots of discussion in comments, no resolution.

 * Cleanup new guice-persist extension.
   Status: Some code review changes applied... is it stable?

 * maven2 POM?
   Status: I don't know?

 * Include guice-grapher extension in release
   Status: Open question, awaiting comments.

Closed issues:
 * Undeprecate Guice.createInjector.  [DONE, r1180]
 * Fix servlet extension to allow multiple modules/filters/injectors in one JVM. [DONE, r1187]
 * Add "@since 3.0" to new APIs.  [DONE,  r1183]
 * Remove ScopeChecker [DONE, r1179]
 * Refactor internal package (move utility class to .internal.util) [DONE, r1185]
 * Fix overridden @Inject method behavior [DONE, r1177]
 * JIT providers / autobinders [SHELVED]

sam

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

--
You received this message because you are subscribed to the Google Groups "google-guice-dev" group.
To post to this group, send email to google-g...@googlegroups.com.
To unsubscribe from this group, send email to google-guice-d...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.

Reply all
Reply to author
Forward
0 new messages