dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.96k stars 4.03k forks source link

Improve memory managment in async methods #437

Open omariom opened 9 years ago

omariom commented 9 years ago

.NET GC enjoys the ability of JIT to tell it when local references are no longer in use. Here is an example:

private void SyncMethod()
{
    byte[] bytes = new byte[1024];
    var wr = new WeakReference(bytes);

    Console.WriteLine(wr.Target != null);

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();

    Console.WriteLine(wr.Target != null);
}

The output: True False

But with async methods all local references, even those which don't cross await points, are promoted to heap allocated state machine and hang there until the async method is done.

private async Task MethodAsync()
{
    byte[] bytes = new byte[1024];
     var wr = new WeakReference(bytes);

    Console.WriteLine(wr.Target != null);

    await Task.Delay(100);

    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();

    Console.WriteLine(wr.Target != null);
}

As async methods tend to run longer than sync ones it can lead to pressure on GC.

The solution could be:

  1. Local reference variables that don't cross await points should not be part of state machine.
  2. Those that do must be nulled out right before the last await point after which they are not going to be used.

Update: It turned out Roslyn already doesn't promote to state machine those locals which don't cross await points. But my second suggestion is still valid. Here is an example:

private static async Task MethodAsync()
{
    byte[] bytes = new byte[1024];
    var wr = new WeakReference(bytes);

    Console.WriteLine(wr.Target != null);

    await Task.Delay(100);

    Console.WriteLine(bytes.Length);

    FullGC();

    Console.WriteLine(wr.Target != null);

    await Task.Delay(100);

    FullGC();

    Console.WriteLine(wr.Target != null);
}

private static void FullGC()
{
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
}
HaloFour commented 9 years ago

You forgot to null out the bytes variable in your SyncMethod so the array isn't being collected. If you set that variable to null in either method before calling GC.Collect() the array should be collected, even in the asynchronous method.

I can see value in having the compiler check whether the variable is referenced across an await boundary and only promoting that variable to a field in the state machine if it does. Then bytes would implicitly fall out of scope and be eligible for collection regardless of whether it was explicitly set to null.

sharwell commented 9 years ago

Note that for debugging purposes, all of the local variables need to be promoted to fields if optimization is disabled.

theoy commented 9 years ago

Inviting @tmat to weigh in. We have had discussions to change closure heuristics, especially when optimization is disabled for debug builds.

One factor to consider is that even though a local is detected to be unused after a certain await point, in Edit-and-Continue scenarios you may decide again at debug time that you want it :smile:

tmat commented 9 years ago

Roslyn does not lift variables that are not used after await in /optimize+ (Release) build. In Debug build all variables are lifted to the state machine to improve debugging experience and enable EnC.

>csc /optimize+ c.cs
Microsoft (R) Visual C# Compiler version 1.0.0.50212
Copyright (C) Microsoft Corporation. All rights reserved.

>c.exe
True
False

>csc /optimize- c.cs
Microsoft (R) Visual C# Compiler version 1.0.0.50212
Copyright (C) Microsoft Corporation. All rights reserved.

>c.exe
True
True
omariom commented 9 years ago

Hmm .. Did you try it with async method?


From: Tomas Matousekmailto:notifications@github.com Sent: ‎13.‎02.‎2015 20:45 To: dotnet/roslynmailto:roslyn@noreply.github.com Subject: Re: [roslyn] Improve memory managment in async methods (#437)

Roslyn does exactly what you are saying in /optimize+ (Release) build. In Debug build all variables are lifted to the state machine to improve debugging experience and enable EnC.

>csc /optimize+ c.cs
Microsoft (R) Visual C# Compiler version 1.0.0.50212
Copyright (C) Microsoft Corporation. All rights reserved.

>c.exe
True
False

>csc /optimize- c.cs
Microsoft (R) Visual C# Compiler version 1.0.0.50212
Copyright (C) Microsoft Corporation. All rights reserved.

>c.exe
True
True

Reply to this email directly or view it on GitHub: https://github.com/dotnet/roslyn/issues/437#issuecomment-74294527

tmat commented 9 years ago

Yes.

omariom commented 9 years ago

@tmat, you are right. I tried it with VS 2013 (silly of me) :confused:

yume-chan commented 9 years ago

@tmat I'm using Visual Studio 2015 Preview but the debugger cannot see value of fields after await calls.

In debugging build, for this code

static async void TestAsync()
{
    int i = 5;
    await Task.Delay(100);
    i = 10;
    await Task.Delay(200);
    Debugger.Break();
}

Roslyn just creates a field in MoveNext() and sets its value to 5 or 10 at their states, but when Debugger.Break(); hits the field will be its initial value, so debugger only sees 0. (Compiled state machine is like:

void MoveNext()
{
    int i;
    if (state == 0)
    {
        i = 5;
        await Task.Delay(100);
    }
    else if (state == 1)
    {
        i = 10;
        await Task.Delay(200);
    }
    else
    {
        Debugger.Break();
    }
}

Another problem is for this code:

class Disposable: IDisposable { /* ... */ }

static async void TestAsync()
{
    var d = new Disposable();
    using (d)
    {
        await Task.Delay(100);
        Debugger.Break();
    }
}

Although Roslyn includes d in the state machine and Dispose it correctly, debugger cannot see value of d when Debugger.Break(); hits (Debugger says it's null). I don't know its causes but maybe there is something wrong with the debug symbols.

More strange is, if I edit the code to

    using (var d = new Disposable())
    {
        await Task.Delay(100);
        Debugger.Break();
    }

Now the debugger can show value of d correctly.