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

eliding #120

Merged
merged 1 commit into from
Feb 9, 2021
Merged

eliding #120

merged 1 commit into from
Feb 9, 2021

Conversation

SimonCropp
Copy link
Contributor

thoughts on eliding some of those asyncs?

@chrissainty
Copy link
Member

I had to google that one, I'll add it to my word of the day list!

What is the benefit here? It seems to go against the async guidance from David Fowler (reference: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#prefer-asyncawait-over-directly-returning-task)

@SimonCropp
Copy link
Contributor Author

SimonCropp commented Feb 5, 2021

i find it interesting the way async is treated. Much of the advice seems to be based on "people are not capable of understanding it, so dont tech the advances scenarios, just tell people to avoid them". so lets take async out of the picture, and instead use the

Lets say you are doing a helper method that returns all temp files:

Would you do this:

IEnumerable<string> GetTempFiles()
{
    return Directory.EnumerateFiles(Path.GetTempPath());
}

or this:

IEnumerable<string> GetTempFiles()
{
    foreach (var file in Directory.EnumerateFiles(Path.GetTempPath()))
    {
        yield return file;
    }
}

the first has similar behaviors and caveats to a direct Task return. the second has similar behaviors and caveats to await.

For me, with both async and yield state machines, i prefer direct return due to the less code. The effects on perf, memory and assembly size are a positive side effect. The impacts on stack traces and exceptions handling i have not found to be a problem.

But obviously this is just my preference :)

@SimonCropp
Copy link
Contributor Author

also that advice is "AspNetCoreDiagnosticScenarios" and in the aspnet core repo there are 1801 instance of non awaited Task returns. admittedly some of those would be interfaces method definitions and some are returning Task.CompletedTask, but the majority are eliding

@SimonCropp
Copy link
Contributor Author

to specifically answer "What is the benefit here"

  • less IL = smaller assembly + faster JIT + smaller mem footprint when assembly is loaded
  • no state machine = less mem uses + less GC pressure + faster due to no instantiation and execution of state machine

But as i said for me, is i primarily about less code

@chrissainty
Copy link
Member

Thanks for taking the time to give such a detailed response @SimonCropp, it's much appreciated.

To give you some reference around my question, I've never had to write any performance sensitive code so I've just followed the guidance of awaiting rather than returning. The type of performance/memory benefits are just unknown to me. So I really appreciate the content of your response.

Based on what you've said it seems like a good change to make. I'll look at merging this over the weekend

@chrissainty chrissainty added the Maintenance General maintenance label Feb 9, 2021
@chrissainty chrissainty merged commit 9d724b0 into Blazored:main Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance General maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants