Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Use same Task for Task.CompletedTask and ATMB.Completed #24404

Closed
wants to merge 3 commits into from
Closed

Use same Task for Task.CompletedTask and ATMB.Completed #24404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a945cd1
Use same Task for Task.CompletedTask and ATMB.Completed
benaadams May 5, 2019
0a62c82
Feedback
benaadams May 6, 2019
12b0a79
Feedback 2
benaadams May 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/AsyncMethodBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,6 @@ private void NotifySynchronizationContextOfCompletion()
/// </remarks>
public struct AsyncTaskMethodBuilder
{
/// <summary>A cached VoidTaskResult task used for builders that complete synchronously.</summary>
#if PROJECTN
private static readonly Task<VoidTaskResult> s_cachedCompleted = AsyncTaskCache.CreateCacheableTask<VoidTaskResult>(default(VoidTaskResult));
#else
private static readonly Task<VoidTaskResult> s_cachedCompleted = AsyncTaskMethodBuilder<VoidTaskResult>.s_defaultResultTask;
#endif

/// <summary>The generic builder object to which this non-generic instance delegates.</summary>
private AsyncTaskMethodBuilder<VoidTaskResult> m_builder; // mutable struct: must not be readonly. Debugger depends on the exact name of this field.

Expand Down Expand Up @@ -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.
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member Author

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]


/// <summary>
/// Completes the <see cref="System.Threading.Tasks.Task"/> in the
Expand Down
4 changes: 3 additions & 1 deletion src/System.Private.CoreLib/shared/System/Threading/Tasks/Task.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,8 +1497,10 @@ bool IAsyncResult.CompletedSynchronously
/// </remarks>
public static TaskFactory Factory { get; } = new TaskFactory();

// Is a Task{VoidTaskResult} so it can be shared with AsyncTaskMethodBuilder
internal static readonly Task<VoidTaskResult> s_cachedCompleted = new Task<VoidTaskResult>(false, default, (TaskCreationOptions)InternalTaskOptions.DoNotDispose, default);
/// <summary>Gets a task that's already been completed successfully.</summary>
public static Task CompletedTask { get; } = new Task(false, (TaskCreationOptions)InternalTaskOptions.DoNotDispose, default);
public static Task CompletedTask => s_cachedCompleted;

/// <summary>
/// Provides an event that can be used to wait for completion.
Expand Down