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

[Refactor] Remove some unnecessary async/await #2739

Merged
merged 8 commits into from
Nov 18, 2023

Conversation

dimson-n
Copy link
Contributor

There are many async functions that doesn't really need to be async (e.g. check preconditions and delegate other operations to subsequent call like some helper).

This PR removes some of unnecessary async/await usage in simple cases.

That may cause slightly improve CPU and memory usage and allows to compiler generate more efficient code. Yes, this is micro-optimizations but every little performance issue touches each library user.

Copy link
Member

@Misha-133 Misha-133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Waterball12
Copy link
Contributor

You lose some information for debugging. seems to be fine for overload but in the PR is not always the case. Should consider how worth is slightly improve CPU and memory usage.

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#prefer-asyncawait-over-directly-returning-task

@dimson-n
Copy link
Contributor Author

dimson-n commented Aug 5, 2023

Good remark, but i'm prefer performance in case of frameworks like D.Net is (just imho).
Exception wrapping guarantees may be matter, but almost all tasks awaited inplace here as i can see.

@Misha-133 Misha-133 enabled auto-merge (squash) November 18, 2023 21:27
@Misha-133 Misha-133 merged commit 86655a8 into discord-net:dev Nov 18, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants