1

I have noticed, that the MiniProfiler somehow doesn't dispose/release the Timings leading to a possible memory leak.

I used the DotMemory of Jetbrains to make a snapshot before and after the workload. All of the 1000 Timings still remained in an instance of the MiniProfiler, which i cannot seem to get rid of.

This could lead quite some problems of, for example, someone would profile his database queries which could have quite some length to them.

Minimal-example:

public static void Main()
    {
        try
        {
            //Snapshot here
            Console.ReadKey();
            var mp = MiniProfiler.StartNew();
            var rnd = new Random();
            for (int j = 0; j < 1000; j++)
            {
                using (mp.Step("outer"))
                {
                    rnd.Next();
                }
            }
            
            Console.WriteLine(MiniProfiler.Current.RenderPlainText());

            mp.Stop(true);
            mp = null;

            if (Debugger.IsAttached)
                Console.ReadKey();
                //Snapshot here
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex);
        }
    }

Thanks for any help!

Edited my question for one more example. Thanks @Ed Pavlov, but as soon as I create a bigger example the Timings will still remain. Next example was build with Release Config and executes a sql query against a database. (I deliberately choose a quite big command text to demonstrate the phenomenon)

public static async Task Main()
    {
        try
        {
            Console.ReadLine();
            await DoStuff();
            Console.ReadLine();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex);
        }
    }

    internal static async Task DoStuff()
    {
        var mp = MiniProfiler.StartNew("Test");
        for (int i = 0; i < 5000; i++)
        {
            await Test();
            if (i % 200 == 0) Console.WriteLine(i);
        }
        Console.WriteLine(MiniProfiler.Current?.RenderPlainText());
        await mp.StopAsync(true);
    }

    public static async Task Test()
    {
        var rnd = new Random();
        var txt = CommandFacade.CommandText.Replace("@@ID@@", rnd.Next(3000).ToString());

        using (MiniProfiler.Current.Step("Level 1"))
        using (var conn = GetConnection())
        using (var cmd = new ProfiledDbCommand(new SqlCommand(txt), conn, null))
        {
            await conn.OpenAsync();
            await cmd.ExecuteNonQueryAsync();

            conn.Close();
        }
    }
    public static DbConnection GetConnection()
    {
        DbConnection cnn = new SqlConnection(ConnectionString);
        if (MiniProfiler.Current != null)
        {
            cnn = new ProfiledDbConnection(cnn, MiniProfiler.Current);
        }
        return cnn;
    }

Picture of dotMemory with new example:
DotMemory Overview Screenshot
Screenshot of retained object

TheMike
  • 19
  • 3

2 Answers2

0

Despite you set the variable "mp" to null when your application is built w/o optimization in the "Debug" configuration, compiler generates code which keeps all objects till the end of the method. As you can see on the screenshot both, MiniProfiler and all instances of Timing are held in memory by local variables.

Build your application in the "Release" configuration, or better extract the code into a separate method to imitate you real code better and you will see that there are no these objects in the memory after the method call.

public static void Main()
{
  //Snapshot here
  Console.ReadLine();

  Isolate();

  Console.WriteLine("Ready");
  Console.ReadLine();
  //Snapshot here
}

private static void Isolate()
{
  try
  {
    var mp = MiniProfiler.StartNew();
    var rnd = new Random();
    for (int j = 0; j < 1000; j++)
    {
      using (mp.Step("outer"))
      {
        rnd.Next();
      }
    }

    Console.WriteLine(MiniProfiler.Current.RenderPlainText());

    mp.Stop(true);
    mp = null;
  }
  catch (Exception ex)
  {
    Console.WriteLine(ex);
  }
}

enter image description here

Ed Pavlov
  • 2,353
  • 2
  • 19
  • 25
0

My reproduction of this suggests that the use of async/await is capturing the execution context, which is keeping reference(s) to the values inside the AsyncLocal fields used internally by MiniProfiler.

The issue is not a lack of disposing or MiniProfiler failing to release the timings. Timing is only disposable so that it can be scoped in a convenient way. The actual Timing.Dispose method is simply a call to Timing.Stop.

Looking at the top-level Timing instance shows that it's being held onto by an ExecutionContext and when I dug further it seemed that was being instantiated due to the async/awaits around the SQL methods. When I switched to the synchronous equivalents all the timings would get GC'd, and replacing the SQL logic with a simple await Task.Delay(1) also replicated the original problem.

I poked around a bit and it looks like this might be mitigated if MiniProfiler did something similar to what was done here, but I'm not sure how significant this problem is as my testing showed that many calls to DoStuff would not increase the remaining memory consumption as only the last profiler was kept.

(Tested with .NET 6 & MiniProfiler 4.2.55)

benbryant
  • 1
  • 1
  • Thanks for your insight on this. We only discovered this problem, because our service was thrice the size since the introduction of the MiniProfiler. We have quite big sql-queries, which retain alongside the timings bloating the RAM usage on strings alone. Switching to synchronous methods would be quite the hassle, but I am willing to try it out when I got some air to breathe. – TheMike Jul 07 '22 at 07:28
  • @TheMike Ah I see :( There might be other ways to help that. What version of MiniProfiler are you using? The latest on NuGet is 4.2.22 (which is almost 2 years old) but on the official StackOverflow MyGet feed you can get 4.2.55 (https://www.myget.org/feed/stackoverflow/package/nuget/MiniProfiler) which includes a few performance improvements. You could also trim the SQL strings by replacing the [default formatter](https://github.com/MiniProfiler/dotnet/blob/f7a8f80350151e4e7400563d157a3de24d680fc4/src/MiniProfiler.Shared/SqlFormatters/ISqlFormatter.cs) – benbryant Jul 07 '22 at 11:58