Skip to content

Elide async lambda and use WrappedCallbackAsync directly #13488

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

Closed
wants to merge 1 commit into from

Conversation

HenKun
Copy link

@HenKun HenKun commented May 20, 2025

Fixes #12697

Proposed changes

  • Elides the async lambda in two locations and uses the method group directly

Customer Impact

  • Depending on the rate InvokeAsync is called to update the UI, it can result in a performance improvement
  • Due to avoiding creating a new delegate each time and creating a state machine, memory can be saved and the GC pressure be reduced

Regression?

  • No

Risk

  • Propagation of exceptions from within callback. But since WrappedCallbackAsync wraps callback in a try-catch block this should be save and not change the current behavior.

Test methodology

  • Code review of related code
  • Existing tests should still pass
Microsoft Reviewers: Open in CodeFlow

@HenKun HenKun marked this pull request as ready for review May 20, 2025 19:26
@HenKun HenKun requested a review from a team as a code owner May 20, 2025 19:26
@KlausLoeffelmann KlausLoeffelmann self-requested a review June 4, 2025 00:04
@KlausLoeffelmann KlausLoeffelmann added this to the 10 Preview6 milestone Jun 4, 2025
@KlausLoeffelmann
Copy link
Member

Hey @HenKun,

I got a change around all of this in the making, and see if it make sense to incorporate it there.

Thanks for the support here!
Falls Dir?/Ihnen? noch andere Dinge auffallen, einfach kurz Bescheid sagen, am besten mich direkt im Issue taggen. Danke!!

@HenKun
Copy link
Author

HenKun commented Jun 13, 2025

Hi @KlausLoeffelmann

It should be superseded already by the contributions in #12696 which were pretty good.

"Dir" is of course okay 👍

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.

Avoid state machine in InvokeAsync
2 participants