Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve codegen for large projects #48

Open
martinothamar opened this issue Aug 11, 2022 · 4 comments
Open

Improve codegen for large projects #48

martinothamar opened this issue Aug 11, 2022 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@martinothamar
Copy link
Owner

As per discussion from #7 (comment), performance for large projects is pretty bad for cases where we don't have a statically known type as method parameter (i.e. ValueTask<TResponse> Send<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = default);). There are some easier wins to be had, but none that preserves the performance advantage that are seen on smaller projects.

I've created the Mediator.Benchmarks.Large project which tracks performance for the above mentioned overload when there are 1000 requests in the assembly.

To not delay 2.0 much further I'm going to complete this release first, then start the work on improving the perf situation:

  • Add overloads for the Send methods that will allow requests without responses to be generic arguments. This is a small improvement for i.e. struct cases where we can avoid boxing. ValueTask<Unit> Send<TRequest>(TRequest request, CancellationToken cancellationToken = default) where TRequest : IRequest;
  • Generate IDs per request that can be mapped to linear array access so that we have close to zero overhead handler lookup. This will require request types to be partial. Not sure if this will become a breaking change or if we can keep it optional and fallback to slower message handling. This is a bigger change but will drastically improve the performance for larger projects
@Tornhoof
Copy link
Contributor

This will require request types to be partial. Not sure if this will become a breaking change or if we can keep it optional and fallback to slower message handling.

I prototyped around that a bit:

  • use static virtual interface, with default value -1
  • use custom attributes annotating the request types which the source generates the override static interface impl with the proper Id
  • check if Id is -1, then use existing fallback code, otherwise linear access array code.

This will then be backwards compatible.

@martinothamar
Copy link
Owner Author

Thanks again. Sounds like a good solution! This issue is on the top of my list after 2.0 is fully release, which should be in the next few days hopefully

@martinothamar martinothamar added the enhancement New feature or request label Jun 21, 2023
@martinothamar martinothamar added this to the 3.0 milestone Jun 21, 2023
@Dreamescaper
Copy link

Would it make sense to generate non-generic "Send" methods per each request? I.e. instead of having a single method
ValueTask<TResponse> Send<TResponse>(IRequest<TResponse> request, CancellationToken cancellationToken = default);
generate multiple methods

ValueTask<Pong> Send(Ping request, CancellationToken cancellationToken = default);
ValueTask<WeatherForecasts[]> Send(GetWeatherForecasts request, CancellationToken cancellationToken = default);
etc

Obviously, in that case interfaces IMediator/ISender/IPublisher would be generated as well. Don't know if that's bad (apart from being a breaking change).
Bonus point - there would be a compile failure is the corresponding handler is not found.

@martinothamar
Copy link
Owner Author

We do that already actually, they are generated on the concrete Mediator type (which is kind of awkward to use atm due to namespace and class default name), so you could inject that and use it. There is also a Roslyn diagnostic for messages (requests, commands, queries) that don't have a handler. So the tricky part is making it as efficient in the IMediator cases and support generic requests (which we don't atm) and still keep current features 😄

As a general update - I have a local branch that solves this issue in a bit of a temporary way - I still think interceptors are the way to go, but a dictionary-based approach will probably come as part of 3.0 this summer. Perf wasn't as bad as I'd feared (hopefully it stays that way). I essentially generate switch or dictionary based approach depending on the amount of messages and wether or not lifetime is Singleton to find the most efficient approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants