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

pass large structs by ref for performance #1

Open
bbarry opened this issue Sep 15, 2016 · 2 comments
Open

pass large structs by ref for performance #1

bbarry opened this issue Sep 15, 2016 · 2 comments

Comments

@bbarry
Copy link

bbarry commented Sep 15, 2016

assuming the readme is correct,

public int Method1()
{
    int[] arr = new[] { 1, 2, 3, 4 };
    int q = 2;
    return this.Method1_ProceduralLinq1(arr, q);
}
private int Method1_ProceduralLinq1(int[] _linqitems, int q)
{
    if (_linqitems == null) throw new ArgumentNullException();

    int num = 0;
    for (int i = 0; i < _linqitems.Length; i++)
    {
        if (_linqitems[i] > q)
            num += num2 + 3;
    }
    return num;
}

should instead be:

...
    return this.Method1_ProceduralLinq1(arr, ref q);
}
private int Method1_ProceduralLinq1(int[] _linqitems, ref int q)
...

It doesn't really matter in this particular case, but it could in cases where the type of the closure was a larger struct or was modified. For example

public void Main()
{
    var arr = new[] { 1, 2, 3, 4 };
    var q = 2;
    var l = arr.Where(x => x > q).Select(x => (q+=x) + 3).Last();
    Console.WriteLine("q: {0}, l: {1}", q, l);
}

should output q: 5, l: 8

@antiufo
Copy link
Owner

antiufo commented Sep 15, 2016

Yes, when a variable is modified by the lambda (flow analysis), the parameter is made ref.

https://github.com/antiufo/roslyn-linq-rewrite/blob/master/RoslynLinqRewrite/RoslynLinqRewrite/LinqRewriter.cs#L320
https://github.com/antiufo/roslyn-linq-rewrite/blob/master/RoslynLinqRewrite/RoslynLinqRewrite/LinqRewriter.cs#L453

I just checked with your example and it correctly generates a ref parameter.

@antiufo antiufo closed this as completed Sep 15, 2016
@bbarry
Copy link
Author

bbarry commented Sep 15, 2016

heh yeah I guess I was unclear, it should be ref if it changes OR it is a struct type. Otherwise a copy is made which can be significant if the struct is large (large is relative here; a Guid benefits from being passed by ref in a hot path):

public class Program {
    static void Main(string[] args) {
        var summary = BenchmarkRunner.Run<Tests>();
    }
}
public class Tests
{
    Guid guid = Guid.NewGuid();

    bool ByValFn(Guid g) { return g == guid; }

    [Benchmark]
    public bool ByVal()
    {
        return ByValFn(guid);
    }

    bool ByRefFn(ref Guid g) { return g == guid; }

    [Benchmark]
    public bool ByRef()
    {
        return ByRefFn(ref guid);
    }
}
 Method |    Median |    StdDev |
------- |---------- |---------- |
  ByVal | 4.9627 ns | 0.2178 ns |
  ByRef | 4.5439 ns | 0.1089 ns |

~9% improvement for passing a Guid by reference (numbers via BenchmarkDotNet).

@antiufo antiufo reopened this Sep 15, 2016
@antiufo antiufo changed the title closure variables should be passed by ref pass large structs by ref for performance Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants