Optimize Generic Handler Registration in MediatR

[zachpainter77]

Ok @jbogard I know how much you disliked the eager loading strategy of the first generic handler registration implementation, so I took a shot at making a lazy loaded implementation that only scans the assembly when you request a generic handler service that is not already registered. This on demand approach seems to be exactly what you were describing to me. Please review. :)

Summary

This PR introduces an optimized mechanism for registering generic handlers in MediatR. The current implementation scans all assemblies passed to MediatR during startup to find every possible concrete implementation and service type that satisfies all generic handler constraints. This PR modifies the behavior to scan and register generic handler services on-demand, triggered only when a specific service is requested.

This feature remains opt-in and can be enabled by setting the RegisterGenericHandlers configuration flag to true.

Changes Made

Optimized Generic Handler Registration:

• The registration process now scans assemblies only when a specific service is requested, rather than eagerly scanning all possible types during startup.
• Once a service is resolved, the registration is cached for future requests.

Dynamic Service Provider Integration:

• Introduced dynamic resolution and caching for generic handlers, minimizing the startup overhead.

Backward Compatibility:

• The feature remains optional and is controlled via the RegisterGenericHandlers flag in the MediatR configuration.
• All previous tests are passing.

Code Refactor:

• Removed old ServiceRegistrar logic used for previous eager loading of generic handlers.
• Removed old configuration properties that attempted to restrain the user from soft locking their app.
• Extracted and modularized some key logic for resolving generic handler types.
• Improved readability and maintainability by reducing logic.

Pros

Improved Performance:

• Reduces startup time by avoiding eager scanning of all assemblies for generic handlers.
• Registration occurs only for the requested generic handler service, minimizing unnecessary work.

Lower Memory Overhead:

• Decreases the number of service descriptors stored in the IServiceCollection since only required services are registered.

Scalability:

• Particularly beneficial for applications with large assemblies and many potential generic handlers, as it avoids a full assembly scan.

Maintains Flexibility:

• Keeps the feature opt-in via RegisterGenericHandlers, ensuring no impact on existing applications unless explicitly enabled.

Compatibility:

• Fully backward-compatible with existing configurations and workflows.
• No third party dependencies required.

Cons

Potential Runtime Costs:

• The first request for a generic handler incurs a slight delay due to on-demand scanning and registration.
• Subsequent requests are unaffected due to caching.

Increased Complexity:

• The dynamic registration mechanism adds complexity to the codebase, which may require additional documentation and onboarding for contributors.
• Addition of another nuget package dependency (Microsoft.Extensions.DependencyInjection)

Conclusion

This PR significantly optimizes the generic handler registration process, making it more efficient and scalable while maintaining backward compatibility. By deferring the registration of generic handlers to runtime and caching them for subsequent use, it strikes a balance between performance and flexibility.

[ScarletKuro]

I'm just a random passerby who has used MediatR in the past, but I don't think this improvement is worth adding a whole new dependency to the core:
https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/MediatR.csproj#L37

While it's fine to have the abstraction since the majority of people use it with ASP.NET Core anyway, there are some niche cases where people use MediatR with other DI containers (like SimpleInjector) and the app doesn't have the MSFT DI. In these cases, this MSFT dependency will be an unnecessary burden. Again, this isn't the end of the world, but it's not listed as a con.
Also, this PR contains some commented-out code that shouldn't be in the final PR:
https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L71

There's also some dead code:
https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L15

This part also never can be null:
https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L95-L97
etc

[zachpainter77]

I'm just a random passerby who has used MediatR in the past, but I don't think this improvement is worth adding a whole new dependency to the core:

https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/MediatR.csproj#L37

While it's fine to have the abstraction since the majority of people use it with ASP.NET Core anyway, there are some niche cases where people use MediatR with other DI containers (like SimpleInjector) and the app doesn't have the MSFT DI. In these cases, this MSFT dependency will be an unnecessary burden. Again, this isn't the end of the world, but it's not listed as a con. Also, this PR contains some commented-out code that shouldn't be in the final PR:

https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L71

There's also some dead code:

https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L15

This part also never can be null:

https://github.com/jbogard/MediatR/blob/761256f3b132ded5e5f6a2d33e3c092de6620f3a/src/MediatR/Registration/DynamicServiceProvider.cs#L95-L97

etc

Thanks for your review! much appreciated!

I have fixed the following issues...

• removed property without reference.
• removed commented out code.
• removed redundant check for null open interface type.

I hear what you are saying with the dependency, however, the library is already dependent upon Microsoft.Extensions.DependencyInjection.Abstractions so I don't think this is a big issue for anyone who is already using this library. This library is necessary for MediatR to implement it's own custom wrapper around ServiceProvider. Since the default one cannot add services after it has already been built. The requirement is to be able to add generic request / handler implementations at runtime after the default service provider has been built. I'd imagine any other DI container that provides this functionality would also have this dependency on Microsoft.Extensions.DependencyInjection...

So I'll update the description / summary to include this as a con. But I don't really see how it could that much of a burden for anyone currently using this with another container.

Edit: But if it absolutely cannot have this dependency then I can take a crack at creating a custom implementation that could possibly be used as replacement to the defined ServiceProvider type in Microsoft.Extensions.DependencyInjection. I'll leave that up to @jbogard I guess. :)

Again, Thank you for taking the time to review much appreciated.

Cheers.

[Rennasccenth]

Right on time, I'm looking forward this PR.

Once I finish my assessments and land a job(hope to), I wanna help with this.

@zachpainter77 theres anything that I can help you with?

[zachpainter77]

Not at the moment but stay tuned I guess 😊

…

On Thu, Jan 30, 2025, 4:06 PM Felipe Nunes ***@***.***> wrote: Right on time, I'm looking forward this PR. Once I finish my assessments and land a job(hope to), I wanna help with this. @zachpainter77 <https://github.com/zachpainter77> theres anything that I can help you with? — Reply to this email directly, view it on GitHub <#1093 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACL2QUXMLBS4YWZNLY4ZEMD2NK5ARAVCNFSM6AAAAABVKZI4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRVHE4DONRUG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[github-actions]

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[zachpainter77]

not stale.. This pr implements the on demand generic handler registration that @jbogard was looking for.

[jbogard]

Sorry haven't forgotten about this one, just busy with day job

[zachpainter77]

Sorry haven't forgotten about this one, just busy with day job

No worries! understandable. :)

[jbogard]

I think this is on the right track but still has some issues. The Mediator class uses static concurrent dictionaries to cache things, but in this implementation, nothing's actually ever cached. The transient constructor method in the (the call to AddTransient) is called every time IMediator is requested. You'll need to use a similar method used in the Mediator class to cache.

I'm not sure what issues would arise from the additional dependency. Maybe some extra types show up in IntelliSense?

[zachpainter77]

I think this is on the right track but still has some issues. The Mediator class uses static concurrent dictionaries to cache things, but in this implementation, nothing's actually ever cached. The transient constructor method in the (the call to AddTransient) is called every time IMediator is requested. You'll need to use a similar method used in the Mediator class to cache.

I'm not sure what issues would arise from the additional dependency. Maybe some extra types show up in IntelliSense?

Hmm.. I'm not quite sure I understand what you mean.. The implementation looks for a dependency and if it is not found using the default container it then checks the custom container. If the types are found it activates and resolves the dependency. If not then it adds it to the custom service provider and stores the registration for future use. This in my understanding is cached. So it only calls AddTransient the first time something is not found in either the default service provider or the custom one that registers dependencies during runtime. The registration is stored as long as the application is running I suppose. I'll run through the code to double check this.

EDIT: AHHH!! ok ok I see what you mean now! I will work on this as soon as I get a chance!

[github-actions]

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[zachpainter77]

Haven't had time to work on this quite yet. Keeping the pr alive with comment.

…

On Tue, Apr 22, 2025, 5:07 AM github-actions[bot] ***@***.***> wrote: This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days. — Reply to this email directly, view it on GitHub <#1093 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACL2QUXW5KGRGFI6ZDZKDVD22YWIRAVCNFSM6AAAAABVKZI4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRRGEZDAMZWGE> . You are receiving this because you were mentioned.Message ID: ***@***.***> *github-actions[bot]* left a comment (LuckyPennySoftware/MediatR#1093) <#1093 (comment)> This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days. — Reply to this email directly, view it on GitHub <#1093 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACL2QUXW5KGRGFI6ZDZKDVD22YWIRAVCNFSM6AAAAABVKZI4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRRGEZDAMZWGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[luckyycode]

one thing I noticed is that if you got IMediator as transient/scoped and the DynamicServiceProvider gets created each time and does not cache

[github-actions]

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[zachpainter77]

If anyone would like to tackle this issue be my guest. I've been quite busy lately.

…

On Wed, May 14, 2025, 10:59 AM neko_code ***@***.***> wrote: *luckyycode* left a comment (LuckyPennySoftware/MediatR#1093) <#1093 (comment)> one thing I noticed is that if you got IMediator as transient/scoped and the DynamicServiceProvider gets created each time and does not cache — Reply to this email directly, view it on GitHub <#1093 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACL2QUX3BGCZ3HFSJUJJ3ED26OABVAVCNFSM6AAAAABVKZI4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBRGA4DEOJUGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Arnab-Developer]

I was looking into the code changes and have one question which I have asked here. I am not too much familiar with the codebase, so may be my question makes no sense.

[Arnab-Developer]

You have added parameters (provider and publisher) in the constructor, but those parameters don't have any usage! Why is it needed?

[zachpainter77]

Closing this pull request I have no wish to contribute to this library now that it is becoming commercial.