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:

MediatR/src/MediatR/MediatR.csproj

Line 37 in 761256f

<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />

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:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Line 71 in 761256f

//public IServiceProvider RootProvider { get { return _rootProvider; } }

There's also some dead code:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Line 15 in 761256f

private readonly Type[] RequestHandlerTypes = new Type[] { typeof(IRequestHandler<>), typeof(IRequestHandler<,>) };

This part also never can be null:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Lines 95 to 97 in 761256f

	var openInterface = hasResponseType ? typeof(IRequestHandler<,>) : typeof(IRequestHandler<>);
	if(openInterface != null)

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:

MediatR/src/MediatR/MediatR.csproj

Line 37 in 761256f

<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />

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:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Line 71 in 761256f

//public IServiceProvider RootProvider { get { return _rootProvider; } }

There's also some dead code:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Line 15 in 761256f

private readonly Type[] RequestHandlerTypes = new Type[] { typeof(IRequestHandler<>), typeof(IRequestHandler<,>) };

This part also never can be null:

MediatR/src/MediatR/Registration/DynamicServiceProvider.cs

Lines 95 to 97 in 761256f

	var openInterface = hasResponseType ? typeof(IRequestHandler<,>) : typeof(IRequestHandler<>);
	if(openInterface != null)

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: ***@***.***>