DefaultServiceLifetime.Transient registers Mediator as transient too as opposed to ReadMe

[gfoidl]

ReadMe states

• Transient - handlers registered as transient, IMediator/Mediator/ISender/IPublisher still singleton

source

but I see the code generated as Transient for the Mediator too (version 2.1.7):

services.Add(new SD(typeof(global::Mediator.Mediator), typeof(global::Mediator.Mediator), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.IMediator), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.ISender), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.IPublisher), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));

As DefaultServiceLifetime -> ServiceLifetime in https://github.com/martinothamar/Mediator/pull/24 is that a documentation issue (i.e. need to update the ReadMe -- I can send a PR) and / or is the behavior by intention?

I'd really like to have the Mediator registered as singleton, while the handlers, etc. be transient / scoped.

[TimothyMakkison]

I'd really like to have the Mediator registered as singleton, while the handlers, etc. be transient / scoped

Not sure what the expected behaviour should be, but for the time being you could you preemptively add the services before calling AddMediator. TryAdd will prevent the re-registration of the types.

[gfoidl]

Thanks for the reminder (sometimes I forget the easy workarounds)! Just to re-add the Mediator after AddMediator again (that one isn't a TryAdd), so DI will use the last one registered.

Not sure what the expected behaviour should be

Maybe provide another option to configure that lifetime too? Something like

public sealed class MediatorOptions
{
    public string Namespace { get; set; } = "Mediator";

    public ServiceLifetime MediatorServiceLifetime { get; set; } = ServiceLifetime.Singleton;
    public ServiceLifetime HandlerServiceLifetime  { get; set; } = ServiceLifetime.Scoped;
}

Renaming the ServiceLifetime property (again) is a breaking-change. But as version 3 is under development this could be taken (also as the impact of that change isn't really big, because when updating from v2 -> v3 one has to only a few places to change -- due to source-generation it's not a binary breaking change anyway (?!)).

[martinothamar]

Hmm I think it used to be as documented atleast, maybe there was a bug at some point that required this. Don't remember but I'll try to dig into it

[martinothamar]

Oo yes apparantly this was a bug that was fixed: https://github.com/martinothamar/Mediator/pull/73 I don't think there is any way around that. We could add the configuration option as you suggest but it is kind of a footgun isn't it? I'm not sure, what do you guys think?

I'll have to update the README in any case, thanks!

[gfoidl]

Thanks for finding the PR.

Would it be possible that instead of https://github.com/martinothamar/Mediator/pull/73 for the activation a new servce cope is created for the handlers? Then IMediator/Mediator/ISender/IPublisher could be singletons, whilst the transient / scoped registered handlers are resolved from the created service scope (which isn't the root-scope then)?

[martinothamar]

Right now, if configured as transient, but used/injected from ASP.NET Core, scoped instances of services could (depending on how IMediator is injected) be shared amongst Mediator handlers. If we made a new scope per Mediator invocation we would not really respect wether or not IMediator originates from a scope, if you get what I mean. That feels unexpected to me, and I would think I would get a bug report from that at some point.

Can I ask what is problematic about this behavior, is it the memory allocation? Or is initialization expensive? I honestly have not looked too much at perf for transient configuration apart from that one optimization which had to be removed

[gfoidl]

if you get what I mean

Yep, I understand, and that's a very valid point.

is it the memory allocation?

Yes, but TBH there are larger allocations (not from Mediator) that should be elided first. So more or less a micro-optimization.

For the scenario w/ ASP.NET Core's request there are some way to differentiate if it's the root scope or not (at simplest, just a reference compare), but now I believe that this adds more overhead and for sure complicates the SGen very much.

Thus the current (documented) behavior is fine for me. When I have more time I'd like to dig into the code and play a bit around to try the (very rough) outline from the last paragraph, and see if that's a) reliable and b) shows up in profiles.

[zyofeng]

While I understand the benefit of reduced allocations for using singleton. This shouldn't be the default, especially when mediators are often used in a vertical slice architecture. For example ef core dbcontext and fluentvalidations are registered as scoped service. So by default handlers would've been incompatible.

On the other hand with transient lifetime the performance difference is negligible compare to the existing MediaR.

Overall I'm not sure what should be the right approach.

[martinothamar]

Doing singleton by default was very intentional. I had 2 design goals initially with this - one was AOT support and the other performance - performance in the sense that you get the most efficient implementation by default, and rather explicitly opt/configure into slower implementations if that is what is wanted.

As for your example, I'm not sure I understand the connection to vertical slice architecture? I think the issue you describe with Scoped services can happen in any application architecture? My recommendation there is to minimize use of scoped lifetime in general. For EFCore I choose DbContextFactory, for HttpContext I just inject IHttpContextAccessor etc etc. The reason why I do that is because Scoped creates some unfortunate coupling and leads to pointless allocations:

• Coupling: Scoped is viral, so if you inject a scoped service, all consuming services have to care about that and either
  • Be scoped themselves (and somewhere up the stack there has to be a scope)
  • Create a scope
• Performance: when the consuming service is made scoped to support consuming that dependency, it forces an allocation (in this case, the handler instances which usually just inject dependencies) for something that could be allocated once. And this waste scales with number of requests, creating pointless churn for the GC. Let's say that for HTTP requests in an application there are on average 3 handlers/pipeline behaviors invoked downstream for each request. Each handler has on average 3 dependencies injected. If we ignore class overhead (object header, method table ptr), that leaves us with (8 * 3) * 3 = 72 bytes allocated per request just to create objects that have 1 method on them invoked. Now that may not be a big deal for a lot of applications, but it is pure waste, which is why I avoid it when I can. This principle is followed in Mediator as well, only consume resources that are actually needed by default, but expose configuration to do whatever you want as well

If performance wasn't a differentiator for Mediator, I'm not sure I would have bothered with making/maintaining this to be honest.

TLDR: by principle Mediator uses the minimum possible resources (e.g. memory) needed by default, as that is what I think software in general should do - be efficient/minimal but flexible/scalable. This princple is sometimes called mechanical sympathy, or non-pessimization

As usual happy to hear different perspectives and opinions on this

[zyofeng]

Reason why I am bringing up vertical slice is sharing a single instance of an infrastructure service is quite different from a request handler. The latter feels like it's going against the principal of segregation.

But overall your argument is valid, for context I am using Mediator in a HotChocolate/GraphQl project which already employs IDbContextFactory, Cosmos Client, IHttpContextAccessor, HybridCache and TimeProvider etc, which all happen to be Singleton, switching from Transient MediatR to Singleton Mediator saw a 10% reduction in real world response time.