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

Question: Why MediatR does not support ValueTasks ? #1046

Open
AkhmedovEhson opened this issue Jun 25, 2024 · 8 comments
Open

Question: Why MediatR does not support ValueTasks ? #1046

AkhmedovEhson opened this issue Jun 25, 2024 · 8 comments

Comments

@AkhmedovEhson
Copy link

Hi there!

First of all, I'd like to commend the team for the excellent work on MediatR. It's more than just a library, it's a well-defined pattern that has significantly improved the structure and maintainability of the projects.

However, I have encountered a performance issue when using MediatR in projects with a strong dependency on it. Specifically, when handling synchronous operations, we are required to return a Task, which introduces unnecessary garbage collection overhead and negatively impacts performance.

To mitigate this, would it be possible for MediatR to support ValueTask in addition to Task? This change could significantly enhance performance for synchronous operations by reducing the overhead associated with task allocation and garbage collection.

Thank you for considering this enhancement. Your work on MediatR is highly appreciated !

@AkhmedovEhson AkhmedovEhson changed the title Why MediatR does not support ValueTasks ? Question: Why MediatR does not support ValueTasks ? Jun 25, 2024
@jbogard
Copy link
Owner

jbogard commented Jul 22, 2024

What would this look like in terms of the API? Do you have a suggestion here?

@grosch-intl
Copy link

I would love to see something like HandleValueTaskAsync that returned a ValueTask

@serber
Copy link

serber commented Sep 3, 2024

@jbogard, in terms of the MediatR API, I think this would require a new interface to define a request handler that returns a ValueTask.

Outline code

public interface IValueTaskRequestHandler<in TRequest, TResponse>
    where TRequest : IRequest<TResponse>
{
    ValueTask<TResponse> Handle(TRequest request, CancellationToken cancellationToken);
}

And add some overhead or create new ValueTaskRequestHandlerWrapper for ValueTask.

Do you see any other options, and how necessary is this enhancement?

Copy link

github-actions bot commented Nov 3, 2024

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

@github-actions github-actions bot added the Stale label Nov 3, 2024
@driekus77
Copy link
Contributor

Is IValueTaskRequestHandler still a todo? Still wanted?

@github-actions github-actions bot removed the Stale label Nov 6, 2024
@remcoros
Copy link
Contributor

remcoros commented Nov 6, 2024

Having ValueTask in Mediatr does not make sense to me.

If you need the performance benefit of ValueTask when doing synchronous work only in a hot path, why do you use Mediatr in that hot path in the first place? You get instantly more performance benefits when cutting out Mediatr vs using ValueTask.

This request feels more like "because we can", not that it actually has any practical use-case.

@AkhmedovEhson
Copy link
Author

Hello @remcoros .

There are many projects are built in CleanArchitecture, with specific patterns.
If we use what "we can", actually we can break architecture principles (Behaviors, Notifications,Handlers, PreProcessors,PostProcessors).

@remcoros
Copy link
Contributor

remcoros commented Nov 29, 2024

Hello @remcoros .

There are many projects are built in CleanArchitecture, with specific patterns. If we use what "we can", actually we can break architecture principles (Behaviors, Notifications,Handlers, PreProcessors,PostProcessors).

ValueTask is only to be used in very specific scenarios where based on performance measurements of hot paths it makes sense to use. Otherwise, you should always use Task. This is clearly documented in https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.valuetask-1?view=net-8.0

MediatR in a hot-path like that is an anti-pattern. The reflection and dictionary lookups it does is way way way more overhead then ValueTask will give you back.

Just because you want to follow a pattern, doesn't make it 'clean' or in this case, it doesn't make sense at all. (again, this is clearly documented on ValueTask).

Show me a real world example with performance benchmarks of where a ValueTask returning mediatr handler actually improves performance, and I will change my standpoint.

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

No branches or pull requests

6 participants