On Wed, Jul 28, 2010 at 11:48, Stuart McCulloch <[email protected]> wrote:
> 2010/7/28 Endre Stølsvik <[email protected]> > >> So the "don't proxy" method is basically worthless, then? >> > > well the method does exactly what it says it will do - proxies won't be > used to satisfy circular dependencies, so if there's a circular dependency > then of course it will fail - but at least you'll get early knowledge of > this in development and can then fix it accordingly > The point is that adding a setter method somewhere in the circular dependency cycle should fix the problem, right? Because then you'd start with that object S, which now obviously should be possible to construct (since it doesn't anymore require an object in the constructor which is part of a cycle). Then you'd construct up the other objects, and finally you'd end up with the object which must be set on object S using your newly added setter. This construction order must furthermore be the result whenever *any * object belonging to that circular dependency graph is requested. > > >> 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? >> > > have you tried it in practice? if you find a situation where a graph could > be satisfied without using a proxy and it still fails then I'd log an issue > with a testcase - then at least we have something concrete to discuss > I have not tried it yet, so this is purely from a theoretic standpoint: If the construction of objects still are made as it was, where Guice constructs objects as requested without any graph logic where setters are not treated as a possible way to break cycles, then this method won't achieve anything. Furthermore, since the construction order is indeterminate, you might get "explosions" by doing something seemingly unrelated in some module, or in some class - that is, even if it still should be possible to construct all objects without using proxies (by using the setters (or fields, obviously)). I will try to find time to test the current code. > 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. >> > > but imho that's something best left until after 3.0 is cut, otherwise the > release will be even more delayed... > Well, I'm just sayin':* If* there is no attempt at doing "cycle resolution", then the new don't-proxy method is worthless. (But since Dhanji says that there *is* resolution in place, then maybe it already works?) Kind regards, Endre. >> On Tue, Jul 27, 2010 at 22:55, Sam Berlin <[email protected]> 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 <[email protected]> >>> >>> On Tue, Jul 27, 2010 at 05:06, Dhanji R. Prasanna <[email protected]>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 <[email protected]> >>>>> >>>>> 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 <[email protected]> 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 <[email protected]> >>>>>>> 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 < <[email protected]> >>>>>>> [email protected]> 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 < <[email protected]>[email protected]> >>>>>>>> >>>>>>>> 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 < <[email protected]> >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> Here's another status update (in an effort to get 3.0 out soon) -- >>>>>>>>>> >>>>>>>>>> Open issues: >>>>>>>>>> >>>>>>>>>> * 366<http://code.google.com/p/google-guice/issues/detail?id=366>- >>>>>>>>>> @Provides doesn't respect @Nullable >>>>>>>>>> Status: Patch attached [from me], needs review (it adds APIs). >>>>>>>>>> >>>>>>>>>> * 436<http://code.google.com/p/google-guice/issues/detail?id=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<http://code.google.com/p/google-guice/issues/detail?id=508>- >>>>>>>>>> AOP visibility checks should include the method return type. >>>>>>>>>> Status: Patch attached [from Stuart], just waiting on some >>>>>>>>>> tests before applying. >>>>>>>>>> >>>>>>>>>> * 509<http://code.google.com/p/google-guice/issues/detail?id=509>- >>>>>>>>>> Cache result of Class.getAnnotations() >>>>>>>>>> Status: Perf improvement request, no patch attached. >>>>>>>>>> >>>>>>>>>> * 510<http://code.google.com/p/google-guice/issues/detail?id=510>- >>>>>>>>>> Remove InjectorBuilder >>>>>>>>>> Status: Rationale for keeping InjectorBuilder listed in the >>>>>>>>>> comments along with other possible scenarios, awaiting replies. >>>>>>>>>> >>>>>>>>>> * 513<http://code.google.com/p/google-guice/issues/detail?id=513>- >>>>>>>>>> Why do we need Injector.getAllBindings? >>>>>>>>>> Status: Rationale for why it's useful in the comments, awaiting >>>>>>>>>> replies. >>>>>>>>>> >>>>>>>>>> * 514<http://code.google.com/p/google-guice/issues/detail?id=514>- >>>>>>>>>> Do we need Injector.getExistingBinding()? >>>>>>>>>> Status: Rationale for why it's useful (but not strictly >>>>>>>>>> necessary) in the comments, awaiting replies. >>>>>>>>>> >>>>>>>>>> * 515<http://code.google.com/p/google-guice/issues/detail?id=515>- >>>>>>>>>> What are the use cases for @Toolable? >>>>>>>>>> Status: Use cases listed in the comments, awaiting replies. >>>>>>>>>> >>>>>>>>>> * 516<http://code.google.com/p/google-guice/issues/detail?id=516>- >>>>>>>>>> Investigate duplicate binding filtering >>>>>>>>>> Status: Performance implications & rationale explained in >>>>>>>>>> comments, awaiting replies. >>>>>>>>>> >>>>>>>>>> * 517<http://code.google.com/p/google-guice/issues/detail?id=517>- >>>>>>>>>> Fix classloader leak >>>>>>>>>> Status: More questions asked in the comments, awaiting answers. >>>>>>>>>> >>>>>>>>>> * 518<http://code.google.com/p/google-guice/issues/detail?id=518>- >>>>>>>>>> Hide Key.ofType() >>>>>>>>>> Status: Question opened, no replies. Awaiting comments. >>>>>>>>>> >>>>>>>>>> * 519<http://code.google.com/p/google-guice/issues/detail?id=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<http://code.google.com/p/google-guice/source/detail?r=1180> >>>>>>>>>> ] >>>>>>>>>> * Fix servlet extension to allow multiple >>>>>>>>>> modules/filters/injectors in one JVM. [DONE, >>>>>>>>>> r1187<http://code.google.com/p/google-guice/source/detail?r=1187> >>>>>>>>>> ] >>>>>>>>>> * Add "@since 3.0" to new APIs. [DONE, >>>>>>>>>> r1183<http://code.google.com/p/google-guice/source/detail?r=1183> >>>>>>>>>> ] >>>>>>>>>> * Remove ScopeChecker [DONE, >>>>>>>>>> r1179<http://code.google.com/p/google-guice/source/detail?r=1179> >>>>>>>>>> ] >>>>>>>>>> * Refactor internal package (move utility class to >>>>>>>>>> .internal.util) [DONE, >>>>>>>>>> r1185<http://code.google.com/p/google-guice/source/detail?r=1185> >>>>>>>>>> ] >>>>>>>>>> * Fix overridden @Inject method behavior [DONE, >>>>>>>>>> r1177<http://code.google.com/p/google-guice/source/detail?r=1177> >>>>>>>>>> ] >>>>>>>>>> * 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 >>>>>>>>>> <[email protected]> >>>>>>>>>> [email protected]. >>>>>>>>>> To unsubscribe from this group, send email to >>>>>>>>>> <google-guice-dev%[email protected]> >>>>>>>>>> [email protected]. >>>>>>>>>> For more options, visit this group at >>>>>>>>>> <http://groups.google.com/group/google-guice-dev?hl=en> >>>>>>>>>> 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 >>>>>>>>> <[email protected]> >>>>>>>>> [email protected]. >>>>>>>>> To unsubscribe from this group, send email to >>>>>>>>> <google-guice-dev%[email protected]> >>>>>>>>> [email protected]. >>>>>>>>> For more options, visit this group at >>>>>>>>> <http://groups.google.com/group/google-guice-dev?hl=en> >>>>>>>>> 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 >>>>>>>> <[email protected]> >>>>>>>> [email protected]. >>>>>>>> To unsubscribe from this group, send email to >>>>>>>> <google-guice-dev%[email protected]> >>>>>>>> [email protected]. >>>>>>>> For more options, visit this group at >>>>>>>> <http://groups.google.com/group/google-guice-dev?hl=en> >>>>>>>> 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 >>>>>>> [email protected]. >>>>>>> To unsubscribe from this group, send email to >>>>>>> [email protected]. >>>>>>> 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 >>>>>>> [email protected]. >>>>>>> To unsubscribe from this group, send email to >>>>>>> [email protected]<google-guice-dev%[email protected]> >>>>>>> . >>>>>>> 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 >>>>>> [email protected]. >>>>>> To unsubscribe from this group, send email to >>>>>> [email protected]<google-guice-dev%[email protected]> >>>>>> . >>>>>> 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 [email protected] >>>>> . >>>>> To unsubscribe from this group, send email to >>>>> [email protected]<google-guice-dev%[email protected]> >>>>> . >>>>> 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 [email protected]. >>>> To unsubscribe from this group, send email to >>>> [email protected]<google-guice-dev%[email protected]> >>>> . >>>> 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 [email protected]. >>> To unsubscribe from this group, send email to >>> [email protected]<google-guice-dev%[email protected]> >>> . >>> 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 [email protected]. >> To unsubscribe from this group, send email to >> [email protected]<google-guice-dev%[email protected]> >> . >> For more options, visit this group at >> http://groups.google.com/group/google-guice-dev?hl=en. >> > > > > -- > Cheers, Stuart > > -- > You received this message because you are subscribed to the Google Groups > "google-guice-dev" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<google-guice-dev%[email protected]> > . > 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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/google-guice-dev?hl=en.
