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

Unused async/await pairs are removed to optimize and reduce compiled code #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

R0ma-D
Copy link

@R0ma-D R0ma-D commented Apr 3, 2017

If some method can be written without await, then it should be written it without await, and remove the async keyword from the method. A non-async method returning Task is more efficient than an async method returning a value/void. Check here:
http://blog.stephencleary.com/2012/02/async-and-await.html

Simple async method:
public async Task UndirectTaskMethod()
{
return await Task.FromResult(0);
}
generate complex method based on state machine like this:
[DebuggerStepThrough, AsyncStateMachine(typeof(AsyncClassObject.d__1))]
public Task UndirectTaskMethod()
{
AsyncClassObject.d__1 d__ = new AsyncClassObject.d__1();
d__.<>4__this = this;
d__.<>t__builder = AsyncTaskMethodBuilder.Create();
d__.<>1__state = -1;
AsyncTaskMethodBuilder <>t__builder = d__.<>t__builder;
<>t__builder.Start<AsyncClassObject.d__1>(ref d__);
return d__.<>t__builder.Task;
}

@aensidhe
Copy link
Member

Yes, but your change has some implications:

public class A
{
    private IDisposable d;
    public Task F()
    {
        return d.DoAsync();      
    }

    public void Dispose()
    {
         d.Dispose();
    }
}

If you call F, then in some other thread user call Dispose, the caller of F will receive very strange behaviour. If we set async/await in F, there will be either ODI, or he will get result.

Also, I would like to see some benchmarks with and without async. Without them we will not accept this PR.

@R0ma-D
Copy link
Author

R0ma-D commented May 10, 2017

I've attach "benchmark" class. With async keyword it takes 493 ms to create 10000 tasks, and only 4 ms without it.

`class Program
{
private const int DelayInMilliseconds = 1;
private const int Size = 10000;

    static void Main(string[] args)
    {
        Measure(() => Task.FromResult(false));

        GC.Collect();
        Console.Write("Measure async ... ");    //493 ms
        var timeAsync = Measure(() => ExecuteWithAsync());
        Console.WriteLine(timeAsync + " ms");

        GC.Collect();
        Console.Write("Measure not async ... "); //4 ms
        var timeNotAsync = Measure(() => ExecuteWithoutAsync());
        Console.WriteLine(timeNotAsync + " ms");

        Console.ReadKey();
    }

    public static long Measure(Func<Task> creator)
    {
        Stopwatch stopwatch = new Stopwatch();

        stopwatch.Start();

        Task[] all = new Task[Size];
        for (int i = 0; i < Size; i++)
            all[i] = creator();
        stopwatch.Stop();

        Task.WaitAll(all);

        return stopwatch.ElapsedMilliseconds;
    }

    public static async Task ExecuteWithAsync()
    {
        await Task.Delay(DelayInMilliseconds);
    }

    public static Task ExecuteWithoutAsync()
    {
        return Task.Delay(DelayInMilliseconds);
    }
}`

@aensidhe
Copy link
Member

aensidhe commented May 12, 2017

We are not just "creating" tasks in air. We are making network calls with latencies large enough to be considered. Also, I suggest to use BenchmarkDotNet

I'll try to write a benchmark next week to test it.

How should we alleviate problem with disposable objects?

@R0ma-D
Copy link
Author

R0ma-D commented May 12, 2017

I do not quite understand the problem. What will be unexpected for the end user? May be CancellationToken as input parameter decide problem? Or could you provide an example of d.DoAsync() method?

@R0ma-D
Copy link
Author

R0ma-D commented May 12, 2017

I've attach new benchmark result:
image

Code with BenchmarkDotNet:
`public class Program
{
private const int DelayInMilliseconds = 1;
private const int Size = 10000;

    static void Main(string[] args)
    {
        BenchmarkRunner.Run(typeof(Program), new MethodInfo[] 
        {
            typeof(Program).GetMethod(nameof(ExecuteWithoutAsyncBenchmark)),
            typeof(Program).GetMethod(nameof(ExecuteWithAsyncBenchmark)),
        });
        Console.ReadKey();
    }

    [Benchmark]
    public void ExecuteWithAsyncBenchmark()
    {
        Measure(() => ExecuteWithAsync());
    }

    [Benchmark]
    public void ExecuteWithoutAsyncBenchmark()
    {
        Measure(() => ExecuteWithoutAsync());
    }
    
    public static void Measure(Func<Task> creator)
    {
        Task[] all = new Task[Size];
        for (int i = 0; i < Size; i++)
            all[i] = creator();
    }

    [Benchmark]
    public static async Task ExecuteWithAsync()
    {
        await Task.Delay(DelayInMilliseconds);
    }

    [Benchmark]
    public static Task ExecuteWithoutAsync()
    {
        return Task.Delay(DelayInMilliseconds);
    }
}`

@aensidhe
Copy link
Member

aensidhe commented Jun 7, 2017

So, I finally did the tests. Results shows no improvement on excluding async-await. Sorry, I had a lot of work to do on main job.

Code for non-async-await is at #106. I look forward where can we improve code more, but I think network exchange will remove any difference here.

Async-await

BenchmarkDotNet=v0.10.7, OS=Windows 10 Redstone 2 (10.0.15063)
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215209 Hz, Resolution=311.0218 ns, Timer=TSC
dotnet cli version=1.0.0
  [Host] : .NET Core 4.6.25211.01, 64bit RyuJIT
  Core   : .NET Core 4.6.25211.01, 64bit RyuJIT

Job=Core  Runtime=Core  
Method Mean Error StdDev
Call 438.6 us 13.10 us 37.59 us

Without async-await

BenchmarkDotNet=v0.10.7, OS=Windows 10 Redstone 2 (10.0.15063)
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215209 Hz, Resolution=311.0218 ns, Timer=TSC
dotnet cli version=1.0.0
  [Host] : .NET Core 4.6.25211.01, 64bit RyuJIT
  Core   : .NET Core 4.6.25211.01, 64bit RyuJIT

Job=Core  Runtime=Core  
Method Mean Error StdDev
Call 448.1 us 16.68 us 47.86 us

@R0ma-D
Copy link
Author

R0ma-D commented Jun 8, 2017

It looks like you are right and all possible performance improvement spends by network communication. Thank you for your efforts.

@aensidhe
Copy link
Member

Since we have improvements on our network stack in 0.8.0, I'll redo the tests this week, I hope.

@aensidhe
Copy link
Member

aensidhe commented Nov 8, 2017

Test for one call. 2200 RPS.

BenchmarkDotNet=v0.10.9, OS=Windows 10.0.16299
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215210 Hz, Resolution=311.0217 ns, Timer=TSC
.NET Core SDK=2.0.0
  [Host]     : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT
  netcore1.1 : .NET Core 1.1.2 (Framework 4.6.25211.01), 64bit RyuJIT
  netcore2.0 : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT

Jit=RyuJit  Platform=X64  Runtime=Core  
Toolchain=CoreCsProj  
Method Job Mean Error StdDev Median P95 Scaled ScaledSD Gen 0 Allocated
Redis netcore1.1 349.0 us 6.956 us 9.045 us 347.0 us 364.4 us 1.00 0.00 - 472 B
Tarantool netcore1.1 322.2 us 4.919 us 4.601 us 320.8 us 328.9 us 0.92 0.03 1.4648 3256 B
Redis netcore2.0 347.3 us 22.424 us 65.055 us 313.4 us 447.1 us 1.00 0.00 - 472 B
Tarantool netcore2.0 429.8 us 11.462 us 13.644 us 427.1 us 453.5 us 1.28 0.22 1.0231 3256 B

Batch=100. 61k RPS.

BenchmarkDotNet=v0.10.9, OS=Windows 10.0.16299
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215210 Hz, Resolution=311.0217 ns, Timer=TSC
.NET Core SDK=2.0.0
  [Host]     : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT
  netcore1.1 : .NET Core 1.1.2 (Framework 4.6.25211.01), 64bit RyuJIT
  netcore2.0 : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT

Jit=RyuJit  Platform=X64  Runtime=Core  
Toolchain=CoreCsProj  
Method Job Mean Error StdDev P95 Scaled ScaledSD Gen 0 Gen 1 Allocated
Redis netcore1.1 646.7 us 14.65 us 42.51 us 715.3 us 1.00 0.00 6.8359 - 21.5 KB
Tarantool netcore1.1 1,801.1 us 71.58 us 209.92 us 2,255.3 us 2.80 0.37 125.0586 0.3516 291.98 KB
Redis netcore2.0 677.9 us 13.36 us 30.15 us 724.2 us 1.00 0.00 6.8359 - 21.51 KB
Tarantool netcore2.0 1,597.3 us 29.87 us 31.96 us 1,641.7 us 2.36 0.12 120.2539 1.2305 291.98 KB

Batch=1000, 90815 RPS

BenchmarkDotNet=v0.10.9, OS=Windows 10.0.16299
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215210 Hz, Resolution=311.0217 ns, Timer=TSC
.NET Core SDK=2.0.0
  [Host]     : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT
  netcore1.1 : .NET Core 1.1.2 (Framework 4.6.25211.01), 64bit RyuJIT
  netcore2.0 : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT

Jit=RyuJit  Platform=X64  Runtime=Core  
Toolchain=CoreCsProj  
Method Job Mean Error StdDev P95 Scaled ScaledSD Gen 0 Gen 1 Allocated
Redis netcore1.1 622.8 us 12.43 us 33.40 us 670.6 us 1.00 0.00 6.8359 - 21.51 KB
Tarantool netcore1.1 2,129.7 us 72.78 us 210.00 us 2,499.1 us 3.43 0.38 125.0000 1.2277 291.98 KB
Redis netcore2.0 3,977.7 us 104.78 us 304.00 us 4,526.4 us 1.00 0.00 70.0605 11.3407 230.84 KB
Tarantool netcore2.0 10,615.1 us 210.71 us 321.77 us 11,011.3 us 2.68 0.21 958.6397 426.0110 2914.63 KB

@aensidhe
Copy link
Member

aensidhe commented Nov 8, 2017

BenchmarkDotNet=v0.10.9, OS=Windows 10.0.16299
Processor=Intel Core i5-4590 CPU 3.30GHz (Haswell), ProcessorCount=4
Frequency=3215210 Hz, Resolution=311.0217 ns, Timer=TSC
.NET Core SDK=2.0.0
  [Host]     : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT
  netcore1.1 : .NET Core 1.1.2 (Framework 4.6.25211.01), 64bit RyuJIT
  netcore2.0 : .NET Core 2.0.0 (Framework 4.6.00001.0), 64bit RyuJIT

Jit=RyuJit  Platform=X64  Runtime=Core  
Toolchain=CoreCsProj  
Method Job Mean Error StdDev P95 Scaled ScaledSD Gen 0 Gen 1 Gen 2 Allocated
Redis netcore1.1 3.899 ms 0.0759 ms 0.1159 ms 4.081 ms 1.00 0.00 76.6369 7.4405 - 236254 B
Call netcore1.1 11.544 ms 0.2244 ms 0.2304 ms 11.875 ms 2.96 0.10 986.3782 482.7724 - 2984584 B
Select netcore1.1 NA NA NA NA ? ? N/A N/A N/A N/A
Redis netcore2.0 3.671 ms 0.0732 ms 0.0685 ms 3.791 ms 1.00 0.00 72.9167 14.0625 - 236213 B
Call netcore2.0 10.655 ms 0.1861 ms 0.1741 ms 10.952 ms 2.90 0.07 952.0833 429.1667 - 2984584 B
Select netcore2.0 11.069 ms 0.1976 ms 0.1848 ms 11.349 ms 3.02 0.07 1532.7381 733.2589 89.2857 4480584 B

Benchmarks with issues:
IncrementBatchBenchmark.Select: netcore1.1(Jit=RyuJit, Platform=X64, Runtime=Core, Toolchain=CoreCsProj)

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

Successfully merging this pull request may close these issues.

2 participants