-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Use same Task for Task.CompletedTask and ATMB.Completed #24404
Conversation
src/System.Private.CoreLib/shared/System/Threading/Tasks/Task.cs
Outdated
Show resolved
Hide resolved
@@ -270,7 +263,7 @@ public Task Task | |||
/// </summary> | |||
/// <exception cref="System.InvalidOperationException">The builder is not initialized.</exception> | |||
/// <exception cref="System.InvalidOperationException">The task has already completed.</exception> | |||
public void SetResult() => m_builder.SetResult(s_cachedCompleted); // Using s_cachedCompleted is faster than using s_defaultResultTask. | |||
public void SetResult() => m_builder.SetResult(Task.s_cachedCompleted); // Using s_cachedCompleted is faster than using s_defaultResultTask via generic. |
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.
Have you confirmed the asm for this is just as good?
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.
Yep, ends up identical
[12 IL=0141 TR=000050 060036BE] [profitable inline] AsyncTaskMethodBuilder:SetResult():this
[13 IL=0011 TR=000240 060036D0] [profitable inline] AsyncTaskMethodBuilder`1:SetResult(ref):this
[0 IL=0026 TR=000260 060036CE] [FAILED: too many basic blocks] AsyncTaskMethodBuilder`1:SetExistingTaskResult(struct):this
...
; Total bytes of code 339, prolog size 29 for method <Main>d__0:MoveNext():this
Its going via a generic rather than non-generic that causes the speed bump
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.
Yep, ends up identical
More or less
- BAA4040000 mov edx, 0x4A4
+ BA84020000 mov edx, 644
E89B83B25F call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
- 48BA80190AD3E5010000 mov rdx, 0x1E5D30A1980
+ 48BA381647D6FC010000 mov rdx, 0x1FCD6471638
488B12 mov rdx, gword ptr [rdx]
CI looks quite broken
|
src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/AsyncMethodBuilder.cs
Outdated
Show resolved
Hide resolved
Code gen ends up identical in usage (other than referring to same object) and a change in .cctors
|
Follow up that takes this a bit further #24431 |
Closing in preference of #24431 |
Keep it hotter in cache rather than alternating between the two for manually and compiler generated async methods
/cc @stephentoub