7

I have a long running SQL query inside a page that I've sped up by using an async task:

using System.Threading.Tasks;
...

var asyncTask = new Task<ResultClass>(
    () =>
    {
        using (var stepAsync = MiniProfiler.Current.Step("Async!"))
        {
            // exec long running SQL
        }
    });

asyncTask.Start();

// do lots of other slow stuff

ResultClass result;
using (var stepWait = MiniProfiler.Current.Step("Wait for Async"))
{
    result = asyncTask.Result;
}

(Note that this syntax will be a lot nicer once C# 5 comes out with async and await)

When using MVC mini profiler I get the timing for "Wait for Async", but I can't get the timing for the "Async!" step.

Is there any way to get those results (maybe just the SQL timings) into the trace for the completed page?

Update

I've found a way to get the profiler steps into the async method:

var asyncTask = new Task<ResultClass>(
    profiler =>
    {
        using (var step = (profiler as MiniProfiler).Step("Async!"))
        {
            // exec long running SQL
        }
    }, MiniProfiler.Current);

That almost works, in that the "Async!" step appears (somewhat randomly, depending on the execution, and with some times appearing as negative) but isn't really what I want. The SQL timings and statements are still lost, and in this case they're the most valuable information.

Ideally I'd like the "Wait for Async" step to be linked to the timings (rather than the start step). Is there some way that stepWait could be linked to the SQL profiler times for the result?

Any ideas?

Keith
  • 150,284
  • 78
  • 298
  • 434
  • 1
    tricky, have you tried defining a new profiling context inside your async task, then you can get a totally new timing for it. – Sam Saffron Dec 18 '11 at 09:46
  • @SamSaffron - Ok, tried that (with something similar to Andres's answer below, but generating the new context inside the Task's lambda). It gets the SQL timings correct but reports multiple async steps together, all with the same duration (about 100ms longer than the slowest SQL statement). Is there any way to get the SQL timings out of the general ones and associate them with my "Wait for Async" step? – Keith Dec 19 '11 at 16:54
  • @SamSaffron - never mind, I think I've figured it out, see below :) – Keith Dec 20 '11 at 10:47

2 Answers2

10

I've found a way to do this, by only keeping the SQL timings the main page steps still add up right:

var asyncTask = new Task<T>(
    profiler =>
    {
        var currentProfiler = (profiler as MiniProfiler);

        // Create a new profiler just for this step, we're only going to use the SQL timings
        MiniProfiler newProfiler = null;
        if (currentProfiler != null)
        {
            newProfiler = new MiniProfiler("Async step", currentProfiler.Level);
        }

        using(var con = /* create new DB connection */)
        using(var profiledCon = new ProfiledDbConnection(con, newProfiler))
        {
            // ### Do long running SQL stuff ###
            profiledCon.Query...
        }

        // If we have a profiler and a current step
        if (currentProfiler != null && currentProfiler.Head != null)
        {
            // Add the SQL timings to the step that's active when the SQL completes
            var currentStep = currentProfiler.Head;
            foreach (var sqlTiming in newProfiler.GetSqlTimings())
            {
                currentStep.AddSqlTiming(sqlTiming);
            }
        }

        return result;
    }, MiniProfiler.Current);

This results in the SQL timings for the long running query being associated with the current step when the SQL completes. Typically this is the step waiting for the async result, but will be an earlier step if the SQL completes before I have to wait for this.

I've wrapped this in a dapper-style QueryAsync<T> extension method (always buffered and not supporting transactions) though it could do with a lot of tidy up. When I have more time I'll look into adding a ProfiledTask<T> or similar that allows the profiled results to be copied from the completed task.

Update 1 (works in 1.9)

Following Sam's comment (see below) he's quite right: AddSqlTiming is not thread safe. So to get around that I've moved that to a synchronous continuation:

// explicit result class for the first task
class ProfiledResult<T>
{
    internal List<SqlTiming> SqlTimings { get; set; }
    internal T Result { get; set; }
}

var currentStep = MiniProfiler.Current.Head;

// Create a task that has its own profiler
var asyncTask = new Task<ProfiledResult<T>>(
    () =>
    {
        // Create a new profiler just for this step, we're only going to use the SQL timings
        var newProfiler = new MiniProfiler("Async step");
        var result = new ProfiledResult<T>();

        result.Result = // ### Do long running SQL stuff ###

        // Get the SQL timing results
        result.SqlTimings = newProfiler.GetSqlTimings();
        return result;
    });

// When the task finishes continue on the main thread to add the SQL timings
var asyncWaiter = asyncTask.ContinueWith<T>(
    t =>
    {
        // Get the wrapped result and add the timings from SQL to the current step
        var completedResult = t.Result;
        foreach (var sqlTiming in completedResult.SqlTimings)
        {
            currentStep.AddSqlTiming(sqlTiming);
        }

        return completedResult.Result;
    }, TaskContinuationOptions.ExecuteSynchronously);


asyncTask.Start();

return asyncWaiter;

This works in MvcMiniProfiler 1.9, but doesn't work in MiniProfiler 2...

Update 2: MiniProfiler >=2

The EF stuff added in version 2 breaks my hack above (it adds an internal-only IsActive flag), meaning that I needed a new approach: a new implementation of BaseProfilerProvider for async tasks:

public class TaskProfilerProvider<T> :
    BaseProfilerProvider
{
    Timing step;
    MiniProfiler asyncProfiler;

    public TaskProfilerProvider(Timing parentStep)
    {
        this.step = parentStep;
    }

    internal T Result { get; set; }

    public override MiniProfiler GetCurrentProfiler()
    {
        return this.asyncProfiler;
    }

    public override MiniProfiler Start(ProfileLevel level)
    {
        var result = new MiniProfiler("TaskProfilerProvider<" + typeof(T).Name + ">", level);
        this.asyncProfiler = result;

        BaseProfilerProvider.SetProfilerActive(result);

        return result;
    }

    public override void Stop(bool discardResults)
    {
        if (this.asyncProfiler == null)
        {
            return;
        }

        if (!BaseProfilerProvider.StopProfiler(this.asyncProfiler))
        {
            return;
        }

        if (discardResults)
        {
            this.asyncProfiler = null;
            return;
        }

        BaseProfilerProvider.SaveProfiler(this.asyncProfiler);
    }

    public T SaveToParent()
    {
        // Add the timings from SQL to the current step
        var asyncProfiler = this.GetCurrentProfiler();
        foreach (var sqlTiming in asyncProfiler.GetSqlTimings())
        {
            this.step.AddSqlTiming(sqlTiming);
        }

        // Clear the results, they should have been copied to the main thread.
        this.Stop(true);

        return this.Result;
    }

    public static T SaveToParent(Task<TaskProfilerProvider<T>> continuedTask)
    {
        return continuedTask.Result.SaveToParent();
    }
}

So then to use this provider I just need to start it when starting the task, and hook up the continuation synchronously (as before):

// Create a task that has its own profiler
var asyncTask = new Task<TaskProfilerProvider<T>>(
    () =>
    {
        // Use the provider to start a new MiniProfiler
        var result = new TaskProfilerProvider<T>(currentStep);
        var asyncProfiler = result.Start(level);

        result.Result = // ### Do long running SQL stuff ###

        // Get the results
        return result;
    });

// When the task finishes continue on the main thread to add the SQL timings
var asyncWaiter = asyncTask.ContinueWith<T>(
    TaskProfilerProvider<T>.SaveToParent, 
    TaskContinuationOptions.ExecuteSynchronously);

asyncTask.Start();

return asyncWaiter;

Now the SQL timings appear consistently against the step that initiated the async action. The "% in sql" is more than 100% though, that extra 82.4% is the time saved by doing the SQL in parallel.

                   duration (ms)  from start (ms)  query time (ms)
Start ...Async     0.0            +19.0            1 sql    4533.0
Wait for ...Async  4132.3         +421.3
                                                    182.4 % in sql

Ideally I'd have the long running SQL query on the wait step rather than the init step, but I can't see a way to do that without changing the return type of the calling methods to explicitly pass around the timings (which would make the profiler considerably more obtrusive).

Keith
  • 150,284
  • 78
  • 298
  • 434
  • 1
    my concern here is that currentStep.AddSqlTiming(sqlTiming); is probably not thread safe, be sure to read through the implementation. also this will probably insert the data in a random spot, perhaps you want to store the head before running the query. – Sam Saffron Dec 21 '11 at 02:22
  • @SamSaffron - you're quite right, `AddSqlTiming` is not thread safe. I've moved it to a synchronous continuation and updated the answer above. It's still not quite right though. – Keith Dec 21 '11 at 11:54
  • @SamSaffron - Ah, never mind that SQL duration is correct. I have 3 parallel SQL statements, each taking between 1 and 4 seconds. The page completes in under 5 seconds, but the profiler's `DurationMillisecondsInSql` holds (correctly) the sum of the 3 statements (around 7 seconds). So that "182.4% in sql" implies that I'm saving more than 82.4% of the duration with the parallel tasks. – Keith Dec 21 '11 at 14:53
  • 1
    most excellent. I was thinking of the idea of parallel SQL the other day, used smartly it can really help. Was thinking of perhaps adding a parallel extension to Dapper so .Query returns an IEnumerable that is deferred (if the results are not ready it paused to get them from the consumers end) – Sam Saffron Dec 22 '11 at 06:21
  • @SamSaffron - Cheers. I started out with a Dapper style extension but now I'm not sure about it: the `DBConnection` instance generally isn't thread safe, which means that I need a new one for each execution. My `.QueryAsync` method appears as an extension of an instance, but actually creates and disposes of its own copy, which can be a little confusing. – Keith Jan 03 '12 at 09:07
  • I can't seem to get any SqlTimings back when using this solution with Entity Framework 4.3 Code First... Any ideas? – Daniel Lorenz Mar 19 '12 at 14:56
  • @SamSaffron this solution is broken by the change to 2.0.2 - the SQL timing still appears for the step that starts the async event, but the duration appears to be to the next step (rather than the entire SQL duration). The "Wait for ...Async" step now appears with a large gap before it instead. Any way to add the time spent waiting for the SQL query to finish into that gap? – Keith Jul 12 '12 at 13:48
  • @DanielLorenz this fix is broken by version 2 of the profiler, it works in 1.9 (http://nuget.org/packages/MiniProfiler/1.9) – Keith Jul 12 '12 at 13:49
  • @DanielLorenz I've applied a fix above that now works in v2, see the second update above. – Keith Jul 17 '12 at 14:50
  • I've had a bug here - I got a JavaScript error when I clicked on the `1 sql` link, because the queries were not created. After a few hours, I figured I need to manually set `MiniProfiler.Current.HasSqlTimings = true;`. It's possible this is a bug in the profiler, I'm not sure. Other than that, thanks a lot for posting this code. – Kobi Jul 25 '12 at 12:32
  • 1
    @Kobi - I haven't seen that, but I have lots of synchronous SQL timings too (so `MiniProfiler.Current.HasSqlTimings` is always true for me). Are all your SQL calls async? If so this could be a bug, I'm not sure - the step should have `HasSqlTimings` as that just checks the same `List` that `AddSqlTiming` adds to. I think maybe that has to cascade up to the `ParentTiming`, but maybe `HasSqlTimings` should cascade down instead. – Keith Jul 25 '12 at 14:39
  • This no longer works, `GetSqlTimings` and `AddSqlTiming` are removed. – Rudey May 09 '17 at 14:27
  • @RuudLenders it's quite an old answer. I think there's `async` support now in miniprofiler, but I haven't had the time to check it out yet. – Keith May 09 '17 at 14:31
  • @Keith yes, I just realized. MiniProfiler v4 is still in beta but it has `async` support. Thanks. – Rudey May 09 '17 at 14:44
3

What you can do is to create a new profiler and attach it to the web one.

var newProfiler = new MiniProfiler("- Other task (discard this time)", ProfileLevel.Verbose);
MiniProfiler.Current.AddProfilerResults(newProfiler);

var asyncTask = new Task(() =>
{
    using (newProfiler.Step("Async!"))
    {
        Thread.Sleep(500);
        using (newProfiler.Step("Async 2!"))
        {
            Thread.Sleep(1000);
        }
    }
});

asyncTask.Start();

The new profiler will have wrong times in its declaration but the steps are gonna be ok.

Andres
  • 667
  • 1
  • 6
  • 14
  • Thanks, but that has the same effect as my update above where I pass `MiniProfiler.Current` to the task: I get a negative time for the same amount and the SQL profile is lost. – Keith Dec 19 '11 at 15:17
  • Got that almost working with some adjustments - I create the `newProfiler` inside the `Task` lambda, pass `MiniProfiler.Current` through to that, and call `AddProfilerResults` after the step has executed. It works ok with a single task, but multiple tasks at the same time appear to all finish together with the same timings. – Keith Dec 19 '11 at 16:57
  • Are you generating a single `newProfiler` or one for each `Task`? Can you post the new code that you wrote? – Andres Dec 19 '11 at 19:13
  • Yes, but it's not really the profiler steps that I want - async steps mixed up with sync ones are always going to be a bit messy. I want the SQL profiler timings to be in the results, ideally associated with the step that waits for the results. – Keith Dec 20 '11 at 10:23