{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/29064","repository_url":"https://api.github.com/repos/dotnet/roslyn","labels_url":"https://api.github.com/repos/dotnet/roslyn/issues/29064/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/roslyn/issues/29064/comments","events_url":"https://api.github.com/repos/dotnet/roslyn/issues/29064/events","html_url":"https://github.com/dotnet/roslyn/issues/29064","id":347554701,"node_id":"MDU6SXNzdWUzNDc1NTQ3MDE=","number":29064,"title":"Optimise async/await and ValueTask","user":{"login":"petroemil","id":13720703,"node_id":"MDQ6VXNlcjEzNzIwNzAz","avatar_url":"https://avatars3.githubusercontent.com/u/13720703?v=4","gravatar_id":"","url":"https://api.github.com/users/petroemil","html_url":"https://github.com/petroemil","followers_url":"https://api.github.com/users/petroemil/followers","following_url":"https://api.github.com/users/petroemil/following{/other_user}","gists_url":"https://api.github.com/users/petroemil/gists{/gist_id}","starred_url":"https://api.github.com/users/petroemil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petroemil/subscriptions","organizations_url":"https://api.github.com/users/petroemil/orgs","repos_url":"https://api.github.com/users/petroemil/repos","events_url":"https://api.github.com/users/petroemil/events{/privacy}","received_events_url":"https://api.github.com/users/petroemil/received_events","type":"User","site_admin":false},"labels":[{"id":166673288,"node_id":"MDU6TGFiZWwxNjY2NzMyODg=","url":"https://api.github.com/repos/dotnet/roslyn/labels/Area-Compilers","name":"Area-Compilers","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-08-03T22:36:12Z","updated_at":"2018-08-07T10:02:20Z","closed_at":null,"author_association":"NONE","body":"**Version Used**: \r\n\r\n.NET Core 2.1\r\n\r\n**Steps to Reproduce**:\r\n\r\nI created a small test application to test the pure overhead of using `Task`/`ValueTask`, `async` keyword in an otherwise synchronous methods and `await`-ing them or using `.Result` or `.GetAwaiter().GetResult()`.\r\n\r\nAs a method signature I had 5 (including the reference) different options:\r\n1.1. `bool Reference() => true;`\r\n1.2. `Task<bool> SyncTask() => Task.FromResult(true);`\r\n1.3. `async Task<bool> AsyncTask() => true;`\r\n1.4. `ValueTask<bool> SyncValueTask() => new ValueTask<bool>(true);`\r\n1.5. `async ValueTask<bool> AsyncValueTask() => true;`\r\n\r\nAnd on the caller side (for the `Task`/`ValueTask` returning options) I had:\r\n2.1. `x.Result`\r\n2.2. `x.GetAwaiter().GetResult()`\r\n2.3. `await x`\r\n2.4. `x.IsCompleted ? x.Result : await x`\r\n2.5. `x.IsCompleted ? x.GetAwaiter().GetResult() : await x`\r\n\r\n**Test results**:\r\n\r\nAs for the `Task` options, using a `Task` in a synchronous way (1.2.), the results were very similar, no matter how I was calling it, and it had (on my machine) a fairly stable ~20x execution time compared to the reference implementation.\r\n\r\nJust by adding an `async` keyword to the method (1.3.), the execution time of the method increased to ~60x, compared to the reference implementation, or ~3x, compared to the non-async version of the method.\r\n\r\nThe `async` version of `ValueTask` method (1.5.) was surprisingly slow, the execution time of such method took - regardless of the calling mechanism - around ~100x, compared to the reference implementation.\r\n\r\nAnd last but not least, the actually interesting bit, the non-async version of the `ValueTask` method (1.4.).\r\nFor most call the execution time was somewhere around ~30x, but when using the `.Result` (2.1. / 2.4.), the execution time dropped down to just ~2-5x, compared to the reference implementation.\r\n\r\n**Expected Behavior**:\r\n\r\nOne of the things that I would like to see is some extra smartness in the compiler to detect methods with `async` keyword in the signature but no `await` in the body and treat them as simple, synchronous methods.\r\n\r\nAnother potential optimisation could be to detect (in compile time) if we are trying to `await` an actually synchronous method in which case the `await` could be just turned into a `.Result` call (or more generically a `.GetAwaiter().GetResult()`).\r\n\r\nAnd last but not least, specifically for `ValueTask` a simple optimisation in the code, like (2.4.) could bring significant performance improvements, minimising the overhead of the method call/execution.\r\n\r\n**PS**:\r\n\r\nI know that these overheads fade away when the method actually has something more complicated in the body, but with things like `System.IO.Pipelines`, the upcoming `IAsyncEnumerable` or even Orleans Grains, we have lots of cases where the contract has to be \"asynchronous\" but the actual implementation won't be and in many cases those methods will be very short and simple so even this kind of optimisation could bring some improvement in the overall performance/throughput of these systems."},"comment":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/comments/411004161","html_url":"https://github.com/dotnet/roslyn/issues/29064#issuecomment-411004161","issue_url":"https://api.github.com/repos/dotnet/roslyn/issues/29064","id":411004161,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMTAwNDE2MQ==","user":{"login":"petroemil","id":13720703,"node_id":"MDQ6VXNlcjEzNzIwNzAz","avatar_url":"https://avatars3.githubusercontent.com/u/13720703?v=4","gravatar_id":"","url":"https://api.github.com/users/petroemil","html_url":"https://github.com/petroemil","followers_url":"https://api.github.com/users/petroemil/followers","following_url":"https://api.github.com/users/petroemil/following{/other_user}","gists_url":"https://api.github.com/users/petroemil/gists{/gist_id}","starred_url":"https://api.github.com/users/petroemil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petroemil/subscriptions","organizations_url":"https://api.github.com/users/petroemil/orgs","repos_url":"https://api.github.com/users/petroemil/repos","events_url":"https://api.github.com/users/petroemil/events{/privacy}","received_events_url":"https://api.github.com/users/petroemil/received_events","type":"User","site_admin":false},"created_at":"2018-08-07T10:02:20Z","updated_at":"2018-08-07T10:02:20Z","author_association":"NONE","body":"Oooh now I feel a bit stupid. It's probably because I never really interact with Tasks other than await-ing them, so I never thought about these extra behaviors an async method has. \r\n\r\nI just always hear that don't use async if my method is synchronous, just use Task.FromResult at the end instead, but no one ever mentions that I should also cover exceptions with Task.FromException... and of course as long as the call is await-ed and wrapped in a try-catch block it will make little-to-no difference, but if I use ContinueWith or Rx to build asynchronous \"pipelines\" this behavior is absolutely essential, but I'm pretty sure a lot of code breaks it.\r\n\r\nThank you for the answers, I think everything got answered, so the issue can probably be closed."}}
{"actor":{"display_login":"petroemil"}}