Change NotifyAsync to Task from ValueTask

ValueTask is more important in high-perf scenarios, which is hardly the case for business logic and app architecture.

Author: kzu

src/Merq.Core/MessageBus.cs

@@ -324,7 +324,7 @@ public System.Collections.Generic.IAsyncEnumerable<TResult> ExecuteStream<TResul
 #endif
 
     /// <inheritdoc/>
-    public ValueTask NotifyAsync<TEvent>(TEvent e, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default)
+    public Task NotifyAsync<TEvent>(TEvent e, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default)
     {
         var type = (e ?? throw new ArgumentNullException(nameof(e))).GetType();
         using var activity = StartEventActivity(type, e, callerName, callerFile, callerLine);
@@ -372,7 +372,7 @@ public ValueTask NotifyAsync<TEvent>(TEvent e, [CallerMemberName] string? caller
             Publishing.Record(watch.ElapsedMilliseconds, new Tag("Event", type.FullName));
         }
 
-        return new ValueTask();
+        return Task.CompletedTask;
     }
 
     /// <summary>

src/Merq.Tests/DuckTyping.cs

@@ -21,14 +21,22 @@ public partial record OtherMessageEvent(string Message)
 
 public class DynamicDuckTyping : DuckTyping
 {
-    protected override IMessageBus CreateMessageBus(IServiceProvider? services = null) =>
-        new DynamicallyMessageBus(services ?? new MockServiceProvider());
+    protected override IMessageBus CreateMessageBus(IServiceCollection? services = null)
+    {
+        services ??= new ServiceCollection();
+        services.AddSingleton<IMessageBus>(sp => new DynamicallyMessageBus(sp));
+        return services.BuildServiceProvider().GetRequiredService<IMessageBus>();
+    }
 }
 
 public class AutoMapperDuckTyping : DuckTyping
 {
-    protected override IMessageBus CreateMessageBus(IServiceProvider? services = null) =>
-        new AutoMapperMessageBus(services ?? new MockServiceProvider());
+    protected override IMessageBus CreateMessageBus(IServiceCollection? services = null)
+    {
+        services ??= new ServiceCollection();
+        services.AddSingleton<IMessageBus>(sp => new AutoMapperMessageBus(sp));
+        return services.BuildServiceProvider().GetRequiredService<IMessageBus>();
+    }
 
     [Fact]
     public void ExecuteWithExtraCtorArg()
@@ -37,7 +45,7 @@ public void ExecuteWithExtraCtorArg()
         var services = new ServiceCollection();
         services.AddSingleton(handler.Object);
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.Echo2("Foo");
 
@@ -49,7 +57,7 @@ public void ExecuteWithExtraCtorArg()
 
 public abstract class DuckTyping
 {
-    protected abstract IMessageBus CreateMessageBus(IServiceProvider? services = null);
+    protected abstract IMessageBus CreateMessageBus(IServiceCollection? services = null);
 
 #if NET6_0_OR_GREATER
     [Fact]
@@ -110,7 +118,7 @@ public void CanHandleDuck()
         var services = new ServiceCollection();
         services.AddSingleton<ICommandHandler<Library1::Library.Echo, string>, Library1::Library.EchoHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         Assert.True(bus.CanHandle<Library2::Library.Echo>());
         Assert.True(bus.CanHandle(new Library2::Library.Echo("Foo")));
@@ -122,7 +130,7 @@ public void CanExecuteDuck()
         var services = new ServiceCollection();
         services.AddSingleton<ICommandHandler<Library1::Library.Echo, string>, Library1::Library.EchoHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.Echo("Foo");
 
@@ -135,7 +143,7 @@ public void ExecuteCommand()
         var services = new ServiceCollection();
         services.AddSingleton<ICommandHandler<Library1::Library.Echo, string>, Library1::Library.EchoHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.Echo("Foo");
 
@@ -150,7 +158,7 @@ public void ExecuteNoOpCommand()
         var services = new ServiceCollection();
         services.AddSingleton<ICommandHandler<Library1::Library.NoOp>, Library1::Library.NoOpHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.NoOp();
 
@@ -163,7 +171,7 @@ public async Task ExecuteAsyncCommandAsync()
         var services = new ServiceCollection();
         services.AddSingleton<IAsyncCommandHandler<Library1::Library.EchoAsync, string>, Library1::Library.EchoAsyncHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.EchoAsync("Foo");
 
@@ -178,7 +186,7 @@ public async Task ExecuteNoOpAsyncCommandAsync()
         var services = new ServiceCollection();
         services.AddSingleton<IAsyncCommandHandler<Library1::Library.NoOpAsync>, Library1::Library.NoOpAsyncHandler>();
         services.AddSingleton(typeof(IServiceCollection), _ => services);
-        var bus = CreateMessageBus(services.BuildServiceProvider());
+        var bus = CreateMessageBus(services);
 
         var cmd = new Library2::Library.NoOpAsync();
 

src/Merq/IMessageBus.cs

@@ -103,7 +103,7 @@ public interface IMessageBus
     /// <param name="callerName">Optional calling member name, provided by default by the compiler.</param>
     /// <param name="callerFile">Optional calling file name, provided by default by the compiler.</param>
     /// <param name="callerLine">Optional calling line number, provided by default by the compiler.</param>
-    ValueTask NotifyAsync<TEvent>(TEvent e, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default);
+    Task NotifyAsync<TEvent>(TEvent e, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default);
 
     /// <summary>
     /// Observes the events of a given type <typeparamref name="TEvent"/>.

src/Merq/IMessageBusExtensions.cs

@@ -1,7 +1,6 @@
 using System;
 using System.ComponentModel;
 using System.Runtime.CompilerServices;
-using System.Threading;
 using System.Threading.Tasks;
 
 namespace Merq;
@@ -18,11 +17,7 @@ public static class IMessageBusExtensions
     [EditorBrowsable(EditorBrowsableState.Never)]
     [Obsolete("Use await NotifyAsync instead.")]
     public static void Notify<TEvent>(this IMessageBus bus, TEvent e, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default)
-    {
-        var result = bus.NotifyAsync(e, callerName, callerFile, callerLine);
-        while (!result.IsCompleted)
-            Thread.SpinWait(1);
-    }
+        => bus.NotifyAsync(e, callerName, callerFile, callerLine).Forget();
 
     /// <summary>
     /// Notifies the bus of a new event instance of <typeparamref name="TEvent"/>.
@@ -35,7 +30,7 @@ public static void Notify<TEvent>(this IMessageBus bus, TEvent e, [CallerMemberN
     /// <summary>
     /// Notifies the bus of a new event instance of <typeparamref name="TEvent"/>.
     /// </summary>
-    public static ValueTask NotifyAsync<TEvent>(this IMessageBus bus, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default) where TEvent : new()
+    public static Task NotifyAsync<TEvent>(this IMessageBus bus, [CallerMemberName] string? callerName = default, [CallerFilePath] string? callerFile = default, [CallerLineNumber] int? callerLine = default) where TEvent : new()
         => bus.NotifyAsync(new TEvent(), callerName, callerFile, callerLine);
 
     /// <summary>

src/Merq/PublicAPI.Shipped.txt

@@ -28,10 +28,12 @@ Merq.IMessageBus.Execute(Merq.ICommand! command, string? callerName = null, stri
 Merq.IMessageBus.Execute<TResult>(Merq.ICommand<TResult>! command, string? callerName = null, string? callerFile = null, int? callerLine = null) -> TResult
 Merq.IMessageBus.ExecuteAsync(Merq.IAsyncCommand! command, System.Threading.CancellationToken cancellation = default(System.Threading.CancellationToken), string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.Task!
 Merq.IMessageBus.ExecuteAsync<TResult>(Merq.IAsyncCommand<TResult>! command, System.Threading.CancellationToken cancellation = default(System.Threading.CancellationToken), string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.Task<TResult>!
-Merq.IMessageBus.NotifyAsync<TEvent>(TEvent e, string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.ValueTask
+Merq.IMessageBus.NotifyAsync<TEvent>(TEvent e, string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.Task!
 Merq.IMessageBus.Observe<TEvent>() -> System.IObservable<TEvent>!
 Merq.IMessageBusExtensions
 static Merq.IMessageBusExtensions.Execute<TCommand>(this Merq.IMessageBus! bus, string? callerName = null, string? callerFile = null, int? callerLine = null) -> void
 static Merq.IMessageBusExtensions.Notify<TEvent>(this Merq.IMessageBus! bus, string? callerName = null, string? callerFile = null, int? callerLine = null) -> void
 static Merq.IMessageBusExtensions.Notify<TEvent>(this Merq.IMessageBus! bus, TEvent e, string? callerName = null, string? callerFile = null, int? callerLine = null) -> void
-static Merq.IMessageBusExtensions.NotifyAsync<TEvent>(this Merq.IMessageBus! bus, string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.ValueTask
+static Merq.IMessageBusExtensions.NotifyAsync<TEvent>(this Merq.IMessageBus! bus, string? callerName = null, string? callerFile = null, int? callerLine = null) -> System.Threading.Tasks.Task!
+Merq.TaskExtensions
+static Merq.TaskExtensions.Forget(this System.Threading.Tasks.Task! task) -> void

src/Merq/TaskExtensions.cs

@@ -0,0 +1,40 @@
+using System.Threading.Tasks;
+
+namespace Merq;
+
+/// <summary>
+/// Adds <see cref="Forget(Task)"/> to safely ignore the result of a task execution.
+/// </summary>
+public static class TaskExtensions
+{
+    /// <summary>
+    /// Observes the task to avoid the UnobservedTaskException event to be raised.
+    /// </summary>
+    public static void Forget(this Task task)
+    {
+        // note: this code is inspired by a tweet from Ben Adams: https://twitter.com/ben_a_adams/status/1045060828700037125
+        // Only care about tasks that may fault (not completed) or are faulted,
+        // so fast-path for SuccessfullyCompleted and Canceled tasks.
+        if (!task.IsCompleted || task.IsFaulted)
+        {
+            // use "_" (Discard operation) to remove the warning IDE0058: Because this call is not awaited, execution of the current method continues before the call is completed
+            // https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/functional/discards?WT.mc_id=DT-MVP-5003978#a-standalone-discard
+            _ = ForgetAwaited(task);
+        }
+
+        // Allocate the async/await state machine only when needed for performance reasons.
+        // More info about the state machine: https://blogs.msdn.microsoft.com/seteplia/2017/11/30/dissecting-the-async-methods-in-c/?WT.mc_id=DT-MVP-5003978
+        async static Task ForgetAwaited(Task task)
+        {
+            try
+            {
+                // No need to resume on the original SynchronizationContext, so use ConfigureAwait(false)
+                await task.ConfigureAwait(false);
+            }
+            catch
+            {
+                // Nothing to do here
+            }
+        }
+    }
+}

src/Samples/Library1/Commands.cs

@@ -24,42 +24,29 @@ public void Execute(NoOp command) { }
 }
 
 [Service]
-public class EchoHandler : ICommandHandler<Echo, string>
+public class EchoHandler(IMessageBus bus) : ICommandHandler<Echo, string>
 {
-    readonly IMessageBus? bus;
-
-    public EchoHandler() { }
-
-    public EchoHandler(IMessageBus bus) => this.bus = bus;
-
     public bool CanExecute(Echo command) => !string.IsNullOrEmpty(command.Message);
 
     public string Execute(Echo command)
     {
         if (string.IsNullOrEmpty(command.Message))
             throw new NotSupportedException("Cannot echo an empty or null message");
 
-        bus?.Notify(new OnDidSay(command.Message));
+        bus.NotifyAsync(new OnDidSay(command.Message)).Forget();
         return command.Message;
     }
 }
 
 [Service]
-public class EchoAsyncHandler : IAsyncCommandHandler<EchoAsync, string>
+public class EchoAsyncHandler(IMessageBus bus) : IAsyncCommandHandler<EchoAsync, string>
 {
-    readonly IMessageBus? bus;
-
-    public EchoAsyncHandler() { }
-
-    public EchoAsyncHandler(IMessageBus bus) => this.bus = bus;
-
-
     public bool CanExecute(EchoAsync command) => true;
 
-    public Task<string> ExecuteAsync(EchoAsync command, CancellationToken cancellation = default)
+    public async Task<string> ExecuteAsync(EchoAsync command, CancellationToken cancellation = default)
     {
-        bus?.Notify(new OnDidSay(command.Message));
-        return Task.FromResult(command.Message);
+        await bus.NotifyAsync(new OnDidSay(command.Message));
+        return command.Message;
     }
 }