IDependency instantiated multiple times in a single request

Topics: Core, Writing modules
Developer
Nov 30, 2011 at 4:35 PM

I'd like to know whether this is by design: I've seen that an IDependency implementation's constructor runs multiple times (two times during page generation, and at intervals afterwards without any requests made to the site). Isn't an IDependency supposed to be instantiated only once in a request?

Nov 30, 2011 at 4:44 PM

The "intervals after" will probably be background tasks (indexing, etc.) as it should be. But two times during page generation really shouldn't be happening, and it's something I've independently verified in debug with object ids. Could be there's a reason for it, but otherwise this could have performance implications.

Developer
Nov 30, 2011 at 5:05 PM

I've also though of background tasks but should those dependencies also get instantiated during a background task that surely are not used in it? I see all IDependeny instances getting instantiated multiple times, also those that should nothing to do with any background task.

It bothers me also because performance implications; now although creating a few (dozen) objects that are optimized for relatively cheap instantiating is not a real deal, but it shouldn't be done unnecessary anyway.

Nov 30, 2011 at 5:28 PM

How do you know they're not necessary ... the thing is, to instantiate one dependency, you need to provide all the dependencies to its constructor, and so on for each of their constructors, until you've got an entire dependency graph. The problem is that everything is connected to everything else - it's practically impossible to get any non-trivial dependency without having to instantiate basically everything, even things you won't be using. The only point where the instantiation chain is broken is when using wrappers like Lazy<T> and Work<T>.

Coordinator
Nov 30, 2011 at 5:47 PM

I agree with Pete about dependency chain, Lazy and Work too, and also that they should not be instantiated twice for a single request. Maybe it's not instantiated by the request but one by the request and another one in the background task, during the same request.

Developer
Nov 30, 2011 at 6:05 PM

If drivers are also created during background tasks, as it seems (although not containing anything regarding indexing) then it would explain why those classes are also instantiated that I've described as unnecessary.

I tried again with a very simple example, a class that implemented an IDependency derivation was the dependeny, and a controller was the consumer. The problem hasn't arose.

Drivers are also instantiated twice.

It should be noted that this double instantiation only happens when cold-starting, not on subsequent request (when everything's fine).

Nov 30, 2011 at 7:11 PM
Edited Nov 30, 2011 at 7:15 PM

In that case it's probably when IOrchardShellEvents.Activated fires during startup. This all happens before the first request has actually come in, so it's a different context and therefore correctly has separate scope.

I'm now sure if you quite follow the implications of what I said about the dependency graph. Let me give a simplest-possible example:

public interface IDependencyA : IDependency
{

  void FooA();
  void Bar();

}

public interface IDependencyB : IDependency
{
  void FooB();
}

public class DependencyA : IDependencyA
{
  private readonly IDependencyB _dependencyB;

  public DependencyA(IDependencyB dependencyB) {
    _dependencyB = dependencyB;
  }

  public void FooA(){
    _dependencyB.FooB();
  }

  public void Bar() {
    Console.WriteLine("Bar");
  }
}

public class DependencyB : IDependencyB
{
  public void FooB(){
    Console.WriteLine("Foo");
  }
  
}

public class MyController : Controller {

  private readonly IDependencyA _dependencyA;

  public MyController(IDependencyA dependencyA) {
    _dependencyA = dependencyA;
  }
  

  public ActionResult MyAction() {
    _dependencyA.Bar();
    return View();
  }
}

So - given the above code, what dependencies must be instanced? Technically, we don't need IDependencyB because we're not calling the FooA method. Unfortunately there's no way for Autofac to know that, it just grabs IDependencyB anyway for DependencyA's constructor. It has to make the B instance *before* it can think about making the A instance.

Well ... there's no huge problem there, we only have to make three instances. But what if DependencyB had an additional function, and this constructor:

public DependencyB(IOrchardServices orchardServices) {
  Services = services;
}

Interesting ... DependencyB is now using IOrchardServices for whatever other functions it performs (functions we are not and should not need to be aware of).

So before Autofac can instance our DependencyB (that we're not even using) it also has to go off and instance an IOrchardServices for us. Let's have a look at that constructor, shall we?

        public OrchardServices(
            IContentManager contentManager,
            ITransactionManager transactionManager,
            IAuthorizer authorizer,
            INotifier notifier,
            Lazy<IShapeFactory> shapeFactory,
            IWorkContextAccessor workContextAccessor) {
            _shapeFactory = shapeFactory;
            _workContextAccessor = workContextAccessor;
            ContentManager = contentManager;
            TransactionManager = transactionManager;
            Authorizer = authorizer;
            Notifier = notifier;
        }

Phew! Now we need all those dependencies, just to get an IOrchardServices, just to have an IDependencyB which we still don't need. And in order for IContentManager to be instanced, we need all the IContentHandlers, and consequently all the IContentPartDrivers.

From this we can learn some things:

a) Anything that injects IOrchardServices (and that's quite a lot of things) will cause all content drivers to be instanced (and consequently most services in the entire application!)

b) If you only need one thing from IOrchardServices it's better to inject just that one thing than the whole IOrchardServices

c) ...Although, it's a somewhat futile quest because it's very hard to do anything in Orchard with at some point hitting IContentManager! (e.g. loading a Site or User object ...)

d) Never perform any work in the constructor! All you should ever be doing is storing the injections (there are some exceptions, such as attaching events in content handlers, but that's still very light work)

Developer
Nov 30, 2011 at 7:38 PM
Edited Nov 30, 2011 at 7:39 PM

Then there is no problem at all. Formerly I haven't recognized that double instantiation only happens on startup, and even then if it's a separate scope from the first request then everything is fine and justified.

Thank you for the well-detailed clarification. Although I've understood the implications of the dependencies forming a complex graph I was not aware of the fact that in the end for an IContentManager instance the drivers are also needed. That really cleared things up, thanks again!

On a related note, slightly offtopic: I was previously wondering whether "method injection" would be possible and/or feasible precisely to remove rarely used dependencies from the constructor. Say, in the above example since DependencyA only needs an IDependenyB in FooA(), we could add an argument to FooA(IDependencyB dependencyB) together with removing the object-wide dependency on IDependencyB. Now such dependencies should also be injected automatically, so method invocation would go through some kind of container too.

Another approach would be to store the instance of the dependency in a public property and mark the method with an attribute that would indicate that it needs an instance filled into the property. This, however, would also need dispatched method invocation.

Nov 30, 2011 at 7:41 PM
Piedone wrote:

On a related note, slightly offtopic: I was previously wondering whether "method injection" would be possible and/or feasible precisely to remove rarely used dependencies from the constructor. Say, in the above example since DependencyA only needs an IDependenyB in FooA(), we could add an argument to FooA(IDependencyB dependencyB) together with removing the object-wide dependency on IDependencyB. Now such dependencies should also be injected automatically, so method invocation would go through some kind of container too.

Another approach would be to store the instance of the dependency in a public property and mark the method with an attribute that would indicate that it needs an instance filled into the property. This, however, would also need dispatched method invocation.

This is exactly what Lazy<T> and Work<T> are for. In the example you can inject Lazy<IDependencyB> instead so it only gets resolved when you need it.

Developer
Nov 30, 2011 at 7:44 PM

Wonderful, thanks.

Developer
Dec 29, 2011 at 11:48 AM

Slightly offtopic again, but ontopic for the previous offtopic: just for the record Lazy<> (and Work<>) retrieves the same instance of ITransientDependency, by design. As I favor constructor injection over Resolve() calls, I've written a small utility to lazy resolve dependencies, even ITransientDependency instances so, that they are also apparent from the constructor (by injecting an IResolve<IMyDependeny>). If anybody knows of something similar please let me know.