simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.22k stars 152 forks source link

A lot of memory usage after migrate from 2.7.1 (400-600MB) to 5.0.0 (1.2 to 1.5 GB) #920

Closed filmar25 closed 3 years ago

filmar25 commented 3 years ago

Hi,

It is absolument possible that I am using the new version of Simple Injector in the wrong way. Because, now after migrate to the new version of Simple Injector, I must change the way to register our types.

I am working with ASP.NET MVC 5 application. Our application integrate those components:

I don't want to use multiple containers to register types, so I am using HybridLifestyle in our Services library :

var lifeStyle = Lifestyle.CreateHybrid(new WebRequestLifestyle(), new ThreadScopedLifestyle());

container.Options.DefaultScopedLifestyle = lifeStyle;
container.Options.DefaultLifestyle = lifeStyle;

var services = typeof(IService).GetChildrenClassesFromSameAssembly().OrderBy(x => x.Name).ToList();
services.ForEach(x => container.Register(x.GetInterfaces().FirstOrDefault(), x, lifeStyle));

container.Collection.Register(typeof(IDomainEventHandler<>),
    new[] { typeof(ConfigurationBase).Assembly }, lifeStyle);

container.Register(typeof(IDomainEventDispatcher), typeof(DomainEventDispatcher), lifeStyle);

container.Register<DbContext>(() =>
{
    var conStr = ConfigurationWrapper.GetConnectionString();
    var database = new SIP.Data.Database(conStr);
    database.SetDomainEventDispatcher(container.GetInstance<IDomainEventDispatcher>());
    return database;
}, lifeStyle);

container.Register(typeof(IRepository<>), typeof(Repository<>), lifeStyle);

And for our Web Application :

var lifeStyle = new WebRequestLifestyle();

container.Register<ISAML, SAML.SAMLLocalFunction>(lifeStyle);

container.RegisterMvcControllers(Assembly.GetExecutingAssembly());

container.RegisterMvcIntegratedFilterProvider();

var services = typeof(IService).GetChildrenClasses(typeof(KioskService).Assembly)
    .OrderBy(x => x.Name).ToList();
services.ForEach(x => container.Register(x.GetInterfaces().FirstOrDefault(), x, lifeStyle));

container.Register<KioskService>(lifeStyle);
container.Register<MobileService>(lifeStyle);
container.Register<Controlperfect.WebServices.Workstation.WorkstationService>(lifeStyle);

container.Register(typeof(PublicComponentHub), () => new PublicComponentHub(), lifeStyle);
container.Register(typeof(ControlperfectComponentHub), () => new ControlperfectComponentHub(), lifeStyle);
container.Register(typeof(DispatchperfectComponentHub), () => new DispatchperfectComponentHub(), lifeStyle);
container.Register(typeof(EquipmentperfectComponentHub), () => new EquipmentperfectComponentHub(), lifeStyle);
container.Register(typeof(KeyperfectComponentHub), () => new KeyperfectComponentHub(), lifeStyle);
container.Register(typeof(ReportperfectComponentHub), () => new ReportperfectComponentHub(), lifeStyle);
container.Register(typeof(WorkstationComponentHub), () => new WorkstationComponentHub(), lifeStyle);

SimpleInjectorServiceHostFactory.SetContainer(ServiceLocator.Container);
DependencyResolver.SetResolver(new SimpleInjectorDependencyResolver(container));

So I don't understand why when the registration happend the memory increase to reach 1.5 GB ?

For the same classes and registration with Simple Injector 2.7.1 we are about 400-600 MB.

Thank you for your help !

dotnetjunkie commented 3 years ago

It is possible that v5 consumes more memory compared to v2, but I find the difference weirdly large. So I suspect another reason, which leads me to the following: I don't see you calling container.Verify() in your code. This might explain the difference in memory consumption between the two versions.

With the introduction of v5, the Container is automatically verified upon first use. This means that, when you call GetInstance for the first time, Simple Injector will do a full verification, just as if you called Container.Verify(). This means though, that at that point of time, all registrations are checked, their expression trees built, compiled to IL, and that IL compiled to machine code.

Do note, however, that this doesn't mean that Simple Injector will consume more memory. It only means that Simple Injector consumes that memory up-front.

Also note that during this verification, generation, and compilation, a lot of GC garbage is produced. So a lot of that memory is gen 0 and will go away after the next GC. So make sure that this memory really stays in use. You can do, for instance, call GC.Collect(); GC.WaitForPendingFinalizers(); as a test and call GC.GetTotalMemory(false) to get the amount of memory that's left. If you're looking at the memory use through the Task Manager, you'll get a very inaccurate picture.

That said, there might be reasons why you want to disable auto verification. This section explains what the downsides are and how to disable it.

If the auto-verification feature was not the cause, please let me know. If you supply me with more detailed information and perhaps some sample code to reproduce the issue, I will certainly take a look at it, and take a look if there's anything to be tweaked.

filmar25 commented 3 years ago

Thank you for your answer!

I suspect my problem come from this registration. Before in 2.7.1 I used this:

container.RegisterManyForOpenGeneric(
    typeof(IDomainEventHandler<>),
    AccessibilityOption.PublicTypesOnly,
    container.RegisterAll,
    typeof(IService).Assembly);

Because those function are not available, now I use :

container.Collection.Register(
    typeof(IDomainEventHandler<>),
    new[] { typeof(ConfigurationBase).Assembly },
    lifeStyle);

All registration made for IDomainEventHandler<> in collection causing creation of Singleton in the Container. May be my problem come from this.

What is the replacement of RegisterOpenGeneric in version 5?

dotnetjunkie commented 3 years ago

What is the replacement of RegisterOpenGeneric in version 5?

Your current use of container.Collection.Register is correct. The main difference between between the two is that you filter by PublicTypesOnly in the previous example, while container.Collection.Register registers internal types as well. But that would typically not yield different results.

I find it unlikely that the change from RegisterManyForOpenGeneric to Collection.Register would cause the problem. Under the covers they do something very similar.

TIP: After you supplied your lifestyle to container.Options.DefaultLifestyle, you won't have to use that lifestyle in your registrations; it has become the default lifestyle instead of Transient. So the following:

container.Options.DefaultLifestyle = lifeStyle;

container.Register(typeof(IDomainEventDispatcher), typeof(DomainEventDispatcher), lifeStyle);

Can be simplified to:

container.Options.DefaultLifestyle = lifeStyle;

container.Register(typeof(IDomainEventDispatcher), typeof(DomainEventDispatcher));
dotnetjunkie commented 3 years ago

I suspect my problem come from this registration.

Why do you expect this to cause the memory issues? I expect the majority of the classes in your configuration to implement IDomainEventHandler<T>. Simply removing that Auto-Registration call will, therefore, remove most of the registrations, and will because of that cause a big drop in memory usage.

Have you tried and tested my suggestions? Questions:

dotnetjunkie commented 3 years ago

Another thing, in the title you are stating that you upgraded to 5.0.0. The release notes, however, advise to "upgrade directly to the latest v5.2.x release." In other words, you should upgrade to v5.3.2.

filmar25 commented 3 years ago

Hello Steven,

Yes I tried all your suggestion, except for Performance Counter :

But I am still worried about my IDomainEventHandler. It still using Singleton after registration :

image

Thank you Steven !

filmar25 commented 3 years ago

Sorry I forgot to mention, I use the last version 5.3.2. I write 5.0.0 because when we go to Reference, it is the version we see. But the package is really 5.3.2.

dotnetjunkie commented 3 years ago

I am doing Container.Verify(). There is no difference between activate or not Auto Verification

Auto verification is effectively the same as Container.Verify(). But is what you're saying is that with v2.7.1 you called Container.Verify() as well?

I am using Task Manager, I check also with Process.GetCurrentProcess().WorkingSet64, and it is the same thing. I use Visual Studio Diagnostic Tools as well. With 2.7.1 I don't have this behavior of memory. If you didn't call Container.Verify() using 2.7.1, set Container.Options.EnableAutoVerification to false (but do call Container.Verify() in a unit or integration test).

You shouldn't use Task Manager to check the memory usage of your application. Task Manager will point at the amount of memory that the process currently claimed from the OS, and doesn't reflect the amount of memory used in .NET. The Task Manager will reflect a recent peak in the application's memory use, but the current amount of used memory can be much lower. This can happen because the application temporarily used a lot of memory For instance, Simple Injector will generate a fair amount of garbage when doing its verification. This will certainly spike the memory use, which will go down, while this shrink will not always be reflected in the Task Manager. With tools as PerfMon, you'll see the actual amount of memory in the heap and you can split it per generation, and see the number of collects.

But I am still worried about my IDomainEventHandler. It still using Singleton after registration :

The screen shot shows something different. It shows that the IEnumerable<IDomainEventHandler<T>> is a singleton, which sounds right, because in Simple Injector, collections are streams, and those streams are singletons. This is in v5, but was also the case in v2. That doesn't mean, though, that the IDomainhandler<T> implementations such stream produces are singleton. They are produced on the fly as described here.

If my tips do not change the situation, would it be possible to provide me with a (stripped down) version of your configuration that produces the same performance characteristics? I might be able to provide you with some code that would generate some output that would allow me to recreate a similar configuration for me to test on.

filmar25 commented 3 years ago

Hello Steven,

Thank you very much for your time and your support. It was very appreciated !

We discovered that our problem didn't come from Simple Injector. It come from Entity Framework. When the first instance of DbContext is created it takes around 700-900MB.

So I close the ticket because it is not Simple Injector problem.

Thank you !

dotnetjunkie commented 3 years ago

Today's lesson: prevent doing big upgrades of multiple libraries in the same release ;-)

filmar25 commented 3 years ago

Today's lesson: prevent doing big upgrades of multiple libraries in the same release ;-)

If it was been, yes, but the problem was switching compile to 64 bits. We still investigate why EF6 take that amount of memory in 64 bits.