-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix nits about the "Async programming scenarios" page #49098
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
base: main
Are you sure you want to change the base?
Fix nits about the "Async programming scenarios" page #49098
Conversation
var html = await s_httpClient.GetStringAsync(URL); | ||
return Regex.Matches(html, @"\.NET").Count; | ||
return await Task.Run(() => | ||
{ | ||
var html = s_httpClient.GetStringAsync(URL).Result; | ||
return Regex.Matches(html, @"\.NET").Count; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the reason for this change. The original code already used an async method that was correctly awaited. Now, it looks like you are using a blocking call is a second thread. I don't see the benefit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I got confused too, by the words: "method should return await". Method can't have the return type of await
, hence I came up with this proposal.
I'm totally OK with reverting it, maybe replacing with some other implementation if you, @BillWagner have an idea of what the requirement can be for this point I mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I'm answering assuming you mean the text in the "Use await inside async() method body".
First a nit: Let's remove the ()
on async. That reads wrong.
The reason you should have an await
expression if you add the async
modifier to a method is efficiency.
Adding the async
modifier requires the compiler to generate the async state machine, which is complicated IL. If you don't need it, it's a fair amount of overhead.
Instead, maybe we should state this as "Only add the async
modifier if the method includes an await
expression.
It's perfectly valid to have a method that returns a Task
or Task<T>
that is synchronous! (Some of the examples in this article could be modified to use that idiom: mainly if the last line of the method is return await <method>
.
There is an important difference in the two idioms: A synchronous method that returns a task type is a normal "synchronous" method. If it has the async
modifier, the return
statement must return the T
(or nothing) and the compiler generates the code that creates the continuation when the asynchronous work completes, and packages the return in a Task
(or ValueTask
) as needed. It also generates the necessary code to create a Faulted
task with the Exception
property in case of failure.
This reverts commit 57fb6e6.
This pull request fixes #47630
It updates the "Async programming scenarios" page as listed in the issue.