0

I have a couple of methods that are provided to me by an API. I wish to write a handy helper to log the execution time of said methods, and any chunk of code in general.

Use case methods would typically look like this :

object       GetData(string statementKey, string parametersJson, out string errorMessage);
Task<object> GetDataAsync(string statementKey, string parametersJson, CancellationToken cancellationToken = default(CancellationToken));

I wrote a method wrapper for the sync method :

public static T With<T>(string message, Func<T> func)
{
    var watch = new Stopwatch();

    T returned;

    watch.Start();
    try
    {
        returned = func.Invoke();
    }
    catch (Exception exception)
    {
        Log.Logger.Error(exception, $"Error in {message}");
        throw;
    }
    finally
    {
        watch.Stop();
    }

    // Logging logic here

    return returned;
}

(I am aware this doesn't work with void methods, but an Action overload is trivial if action is sync).

Now if the passed method is Async, I would measure inaccurate execution times. I am a bit confused about how would I need to change the method above to work with async methods.

I tried this implementation but it feels wrong.

public static async Task<T> AsyncWith<T>(string message, Func<Task<T>> func)
{
    T returned;


    try
    {
        var watch = new Stopwatch();
        watch.Start();
        returned = await func.Invoke().ConfigureAwait(false);
        watch.Stop();
        // Logging logic here
    }
    catch (Exception exception)
    {
        Log.Logger.Error(exception, $"Error in {message}");
        throw;
    }

    return returned;
}

Shouldn't I start a task actually ? I don't understand why it is compiling with T returned instead of Task<T> returned

FooBar
  • 132
  • 1
  • 9
  • 3
    `Shouldn't I start a task actually ?` - the task is supposed to be started by `func`. Technically `func` may return a task that has not started, but that would be *weird*. `I don't understand why it is compiling with T returned instead of Task returned` - because you have marked `AsyncWith` as `async`. In methods marked as `async`, you `return` the type declared inside the `Task`. – GSerg Jan 27 '20 at 17:17
  • 2
    You are [otherwise fine](https://stackoverflow.com/a/8525207/11683). – GSerg Jan 27 '20 at 17:20
  • What do you mean - times are not accurate? awating the task will run watch.Stop() when the task from the func has finnished - which means it will measure the correct execution time for the func. – vasil oreshenski Jan 27 '20 at 17:31
  • @vasiloreshenski If I use the With method I wrote above with an async delegate, the finally block may be executed before the async delegate call is finished, and I would not measure the execution time of the async method. Hence the need for an async method. – FooBar Jan 27 '20 at 18:18
  • @GSerg I get that the async makes me return T for a Task declared return type, I was wondering what happens under the hood for this to happen. I'll peek at IL to see if it can help me understand – FooBar Jan 27 '20 at 18:18
  • @FooBar I understand now. You mean if you pass async method in the .With(..) method you will get incorrect times - which is correct. My mistake, i've read something completely different ;) – vasil oreshenski Jan 27 '20 at 18:26
  • 1
    @FooBar Under the hood, the compiler transforms an async method into a state machine [like this](https://stackoverflow.com/a/56587815/11683). – GSerg Jan 27 '20 at 18:30

1 Answers1

1

I tried this implementation but it feels wrong.

Your implementation is correct.

Shouldn't I start a task actually ?

Methods return their tasks "hot" - i.e., running. So calling func.Invoke() is sufficient to start the task.

I don't understand why it is compiling with T returned instead of Task returned

Because the async keyword handles creating the Task<T> wrapper for you, and converts return statements (or exceptions) into logic that completes the Task<T>.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Thanks. I've a follow-up question. Looking at the method I wrote, I have two ways for the `async` version of `void With(string message, Action action)`. Either the passed method is `async Task` and I can write the signature `async Task AsyncWith(string message, Func func)`, or the method is a dreaded `async void`. For curiosity sake, how would you recommend I handle those here ? I believe it should be something like `async Task AsyncWith(string message, Action action)` that just fire-forget the delegate : `await Task.Run(action.Invoke).ConfigureAwait(false)`. Is it the correct way ? – FooBar Jan 27 '20 at 19:10
  • [The `async` version of `Action` is `Func`](https://blog.stephencleary.com/2014/02/synchronous-and-asynchronous-delegate.html), so use `async Task AsyncWith(string message, Func func)` and use `await func().ConfigureAwait(false);`. There's no need for `Task.Run`. – Stephen Cleary Jan 27 '20 at 22:04