Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async Local function use with locals allocate (even if never called) #18946

Open
benaadams opened this issue Apr 24, 2017 · 1 comment
Open
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature Request
Milestone

Comments

@benaadams
Copy link
Member

benaadams commented Apr 24, 2017

Version Used:

.NET Command Line Tools (2.0.0-preview1-005805)

Product Information:
 Version:            2.0.0-preview1-005805
 Commit SHA-1 hash:  78868ad33a

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.16179
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.0.0-preview1-005805\

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.0-preview1-002021-00
  Build    : e9456614cfe65b6dc7c5326a597337159142c84c

Steps to Reproduce:
https://github.com/benaadams/Issues/tree/master/LocalFunctionAllocations

Pass via params: No allocations

public ValueTask<int> GetTotalQuantityAsync()
{
    {
        int total = 0;
        for (int i = 0; i < Orders.Count; i++)
        {
            var task = Orders[i].GetOrderQuantityAsync();
            if (!task.IsCompletedSuccessfully) return Awaited(task, total, i);
            total += task.Result;
        }
        return new ValueTask<int>(total);
    }

    async ValueTask<int> Awaited(ValueTask<int> task, int total, int i)
    {
        total += await task;
        for (i++; i < Orders.Count; i++)
        {
            task = Orders[i].GetOrderQuantityAsync();
            total += (task.IsCompletedSuccessfully) ? task.Result : await task;
        }
        return total;
    }
}

Pass via locals: Allocations (even when Local function is never called)

public ValueTask<int> GetTotalQuantityLocalsAsync()
{
    int total = 0;
    int i = 0;
    ValueTask<int> task;
    for (; i < Orders.Count; i++)
    {
        task = Orders[i].GetOrderQuantityLocalsAsync();
        if (!task.IsCompletedSuccessfully) return Awaited();
        total += task.Result;
    }
    return new ValueTask<int>(total);

    async ValueTask<int> Awaited()
    {
        total += await task;
        for (i++; i < Orders.Count; i++)
        {
            task = Orders[i].GetOrderQuantityLocalsAsync();
            total += (task.IsCompletedSuccessfully) ? task.Result : await task;
        }
        return total;
    }
}

Expected Behavior:
No allocations

Actual Behavior:
2.45 kB of allocations (and performance impact)

                               Method |          Mean |     StdDev |       Op/s | Scaled | Allocated |
------------------------------------- |--------------:|-----------:|-----------:|-------:|----------:|
                                 Sync |   599.9187 ns |  2.1857 ns | 1666892.40 |   1.00 |      0 kB |
 'ValueTask + Local Async via Params' |   668.2087 ns |  4.8867 ns | 1496538.39 |   1.11 |      0 kB |
 'ValueTask + Local Async via Locals' | 1,525.9064 ns |  9.9561 ns |  655348.19 |   2.54 |   2.45 kB |
               'ValueTask Pure Async' | 5,606.6241 ns | 18.2806 ns |  178360.45 |   9.35 |      0 kB |

The ValueTask path is pretty close to the sync path when passing to the local function async fallback via params; but not when using locals; even though the fallback is never used.

/cc @mgravell @stephentoub @davidfowl

@mgravell
Copy link
Member

To avoid this, I tend to write them as regular methods then relocate them. Awkward, and i should probably just figure out how to write an analyzer to do it for me...

I've also taken to adding an extra (artificial) scope level when using local functions; so the local names don't collide.

Random syntax idea: allow static to be declared, which would disallow all capture (including this, which is perhaps an unwanted side-effect, but meh). Additional additional syntax idea - allow me to add attributes ([MethodImpl] in particular, because I'm evil like that)

@VSadov VSadov assigned VSadov and unassigned agocke Apr 30, 2017
@benaadams benaadams changed the title Local function use with locals allocate (even if never called) async Local function use with locals allocate (even if never called) Oct 19, 2017
@jaredpar jaredpar modified the milestones: 15.6, 15.7, 16.0 Jan 5, 2018
@jinujoseph jinujoseph modified the milestones: 16.0, 16.3 Jun 9, 2019
@jcouv jcouv modified the milestones: 16.3, Compiler.Next Jul 11, 2019
@gafter gafter added Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality Code Gen Quality Room for improvement in the quality of the compiler's generated code and removed Concept-Code Quality Improvement Concept-Design Debt Engineering Debt, Design Debt, or poor product code quality labels Sep 17, 2019
@jaredpar jaredpar modified the milestones: Compiler.Next, Backlog Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature Request
Projects
None yet
Development

No branches or pull requests

9 participants