Streamline AsyncTaskMethodBuilders
authorStephen Toub <stoub@microsoft.com>
Thu, 9 Feb 2017 21:46:03 +0000 (16:46 -0500)
committerStephen Toub <stoub@microsoft.com>
Sat, 11 Feb 2017 12:37:38 +0000 (07:37 -0500)
- Slim down AsyncTaskMethodBuilder`1.Task and make it aggressively inlined.  The common case we care to optimize for is synchronous completion, in which case the first access to Task will already fine m_task non-null, and we want that access inlined.
- Slim down AsyncTaskMethodBuilder`1.SetResult.  As with Task, we care most about the synchronous completion path, as when perf matters, most async method invocations complete synchronously.  The code path is streamlined so that the synchronous completion path can mostly be inlined.
- Replace some throws with ThrowHelper
- Mark GetTaskForResult as aggressive inlining.  It contains a lot of C# code, but for a given TResult, the JIT trims away the majority of the implementation.

Commit migrated from https://github.com/dotnet/coreclr/commit/88a4a25312d091dcb44cae738a49bd8b981303eb

src/coreclr/src/mscorlib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs
src/coreclr/src/mscorlib/src/System/Threading/Tasks/TaskFactory.cs
src/coreclr/src/mscorlib/src/System/ThrowHelper.cs

index afb0c22..7915e19 100644 (file)
@@ -63,7 +63,7 @@ namespace System.Runtime.CompilerServices
             // See comment on AsyncMethodBuilderCore.Start
             // AsyncMethodBuilderCore.Start(ref stateMachine);
 
-            if (stateMachine == null) throw new ArgumentNullException(nameof(stateMachine));
+            if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine);
             Contract.EndContractBlock();
 
             // Run the MoveNext method within a copy-on-write ExecutionContext scope.
@@ -72,7 +72,6 @@ namespace System.Runtime.CompilerServices
 
             Thread currentThread = Thread.CurrentThread;
             ExecutionContextSwitcher ecs = default(ExecutionContextSwitcher);
-            RuntimeHelpers.PrepareConstrainedRegions();
             try
             {
                 ExecutionContext.EstablishCopyOnWriteScope(currentThread, ref ecs);
@@ -240,15 +239,8 @@ namespace System.Runtime.CompilerServices
             }
         }
 
-        // This property lazily instantiates the Task in a non-thread-safe manner.  
-        private Task Task 
-        {
-            get
-            {
-                if (m_task == null) m_task = new Task();
-                return m_task;
-            }
-        }
+        /// <summary>Lazily instantiate the Task in a non-thread-safe manner.</summary>
+        private Task Task => m_task ?? (m_task = new Task());
 
         /// <summary>
         /// Gets an object that may be used to uniquely identify this builder to the debugger.
@@ -295,7 +287,7 @@ namespace System.Runtime.CompilerServices
             // See comment on AsyncMethodBuilderCore.Start
             // AsyncMethodBuilderCore.Start(ref stateMachine);
 
-            if (stateMachine == null) throw new ArgumentNullException(nameof(stateMachine));
+            if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine);
             Contract.EndContractBlock();
 
             // Run the MoveNext method within a copy-on-write ExecutionContext scope.
@@ -304,7 +296,6 @@ namespace System.Runtime.CompilerServices
 
             Thread currentThread = Thread.CurrentThread;
             ExecutionContextSwitcher ecs = default(ExecutionContextSwitcher);
-            RuntimeHelpers.PrepareConstrainedRegions();
             try
             {
                 ExecutionContext.EstablishCopyOnWriteScope(currentThread, ref ecs);
@@ -358,7 +349,11 @@ namespace System.Runtime.CompilerServices
         /// <summary>Gets the <see cref="System.Threading.Tasks.Task"/> for this builder.</summary>
         /// <returns>The <see cref="System.Threading.Tasks.Task"/> representing the builder's asynchronous operation.</returns>
         /// <exception cref="System.InvalidOperationException">The builder is not initialized.</exception>
-        public Task Task { get { return m_builder.Task; } }
+        public Task Task
+        {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            get { return m_builder.Task; }
+        }
 
         /// <summary>
         /// Completes the <see cref="System.Threading.Tasks.Task"/> in the 
@@ -449,7 +444,7 @@ namespace System.Runtime.CompilerServices
             // See comment on AsyncMethodBuilderCore.Start
             // AsyncMethodBuilderCore.Start(ref stateMachine);
 
-            if (stateMachine == null) throw new ArgumentNullException(nameof(stateMachine));
+            if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine);
             Contract.EndContractBlock();
 
             // Run the MoveNext method within a copy-on-write ExecutionContext scope.
@@ -458,7 +453,6 @@ namespace System.Runtime.CompilerServices
 
             Thread currentThread = Thread.CurrentThread;
             ExecutionContextSwitcher ecs = default(ExecutionContextSwitcher);
-            RuntimeHelpers.PrepareConstrainedRegions();
             try
             {
                 ExecutionContext.EstablishCopyOnWriteScope(currentThread, ref ecs);
@@ -502,7 +496,7 @@ namespace System.Runtime.CompilerServices
                 {
                     // Force the Task to be initialized prior to the first suspending await so 
                     // that the original stack-based builder has a reference to the right Task.
-                    var builtTask = this.Task;
+                    Task builtTask = this.Task;
 
                     // Box the state machine, then tell the boxed instance to call back into its own builder,
                     // so we can cache the boxed reference. NOTE: The language compiler may choose to use
@@ -542,7 +536,7 @@ namespace System.Runtime.CompilerServices
                 {
                     // Force the Task to be initialized prior to the first suspending await so 
                     // that the original stack-based builder has a reference to the right Task.
-                    var builtTask = this.Task;
+                    Task<TResult> builtTask = this.Task;
 
                     // Box the state machine, then tell the boxed instance to call back into its own builder,
                     // so we can cache the boxed reference. NOTE: The language compiler may choose to use
@@ -563,15 +557,14 @@ namespace System.Runtime.CompilerServices
         /// <returns>The <see cref="System.Threading.Tasks.Task{TResult}"/> representing the builder's asynchronous operation.</returns>
         public Task<TResult> Task
         {
-            get
-            {
-                // Get and return the task. If there isn't one, first create one and store it.
-                var task = m_task;
-                if (task == null) { m_task = task = new Task<TResult>(); }
-                return task;
-            }
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            get { return m_task ?? InitializeTask(); }
         }
 
+        /// <summary>Initializes the task, which must not yet be initialized.</summary>
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private Task<TResult> InitializeTask() => (m_task = new Task<TResult>());
+
         /// <summary>
         /// Completes the <see cref="System.Threading.Tasks.Task{TResult}"/> in the 
         /// <see cref="System.Threading.Tasks.TaskStatus">RanToCompletion</see> state with the specified result.
@@ -582,28 +575,49 @@ namespace System.Runtime.CompilerServices
         {
             // Get the currently stored task, which will be non-null if get_Task has already been accessed.
             // If there isn't one, get a task and store it.
-            var task = m_task;
-            if (task == null)
+            if (m_task == null)
             {
                 m_task = GetTaskForResult(result);
                 Debug.Assert(m_task != null, "GetTaskForResult should never return null");
             }
-            // Slow path: complete the existing task.
             else
             {
-                if (AsyncCausalityTracer.LoggingOn)
-                    AsyncCausalityTracer.TraceOperationCompletion(CausalityTraceLevel.Required, task.Id, AsyncCausalityStatus.Completed);
+                // Slow path: complete the existing task.
+                SetExistingTaskResult(result);
+            }
+        }
 
-                //only log if we have a real task that was previously created
-                if (System.Threading.Tasks.Task.s_asyncDebuggingEnabled)
-                {
-                    System.Threading.Tasks.Task.RemoveFromActiveTasks(task.Id);
-                }
+        /// <summary>Completes the already initialized task with the specified result.</summary>
+        /// <param name="result">The result to use to complete the task.</param>
+        private void SetExistingTaskResult(TResult result)
+        {
+            Debug.Assert(m_task != null, "Expected non-null task");
 
-                if (!task.TrySetResult(result))
-                {
-                    throw new InvalidOperationException(Environment.GetResourceString("TaskT_TransitionToFinal_AlreadyCompleted"));
-                }
+            if (AsyncCausalityTracer.LoggingOn || System.Threading.Tasks.Task.s_asyncDebuggingEnabled)
+            {
+                LogExistingTaskCompletion();
+            }
+
+            if (!m_task.TrySetResult(result))
+            {
+                ThrowHelper.ThrowInvalidOperationException(ExceptionResource.TaskT_TransitionToFinal_AlreadyCompleted);
+            }
+        }
+
+        /// <summary>Handles logging for the successful completion of an operation.</summary>
+        private void LogExistingTaskCompletion()
+        {
+            Debug.Assert(m_task != null);
+
+            if (AsyncCausalityTracer.LoggingOn)
+            {
+                AsyncCausalityTracer.TraceOperationCompletion(CausalityTraceLevel.Required, m_task.Id, AsyncCausalityStatus.Completed);
+            }
+
+            // only log if we have a real task that was previously created
+            if (System.Threading.Tasks.Task.s_asyncDebuggingEnabled)
+            {
+                System.Threading.Tasks.Task.RemoveFromActiveTasks(m_task.Id);
             }
         }
 
@@ -620,15 +634,14 @@ namespace System.Runtime.CompilerServices
 
             // Get the currently stored task, which will be non-null if get_Task has already been accessed.
             // If there isn't one, store the supplied completed task.
-            var task = m_task;
-            if (task == null)
+            if (m_task == null)
             {
                 m_task = completedTask;
             }
             else
             {
                 // Otherwise, complete the task that's there.
-                SetResult(default(TResult));
+                SetExistingTaskResult(default(TResult));
             }
         }
 
@@ -667,7 +680,7 @@ namespace System.Runtime.CompilerServices
 
             if (!successfullySet)
             {
-                throw new InvalidOperationException(Environment.GetResourceString("TaskT_TransitionToFinal_AlreadyCompleted"));
+                ThrowHelper.ThrowInvalidOperationException(ExceptionResource.TaskT_TransitionToFinal_AlreadyCompleted);
             }
         }
 
@@ -704,6 +717,7 @@ namespace System.Runtime.CompilerServices
         /// </summary>
         /// <param name="result">The result for which we need a task.</param>
         /// <returns>The completed task containing the result.</returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)] // method looks long, but for a given TResult it results in a relatively small amount of asm
         private Task<TResult> GetTaskForResult(TResult result)
         {
             Contract.Ensures(
@@ -846,9 +860,9 @@ namespace System.Runtime.CompilerServices
         /// <exception cref="System.InvalidOperationException">The builder is incorrectly initialized.</exception>
         public void SetStateMachine(IAsyncStateMachine stateMachine)
         {
-            if (stateMachine == null) throw new ArgumentNullException(nameof(stateMachine));
+            if (stateMachine == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.stateMachine);
             Contract.EndContractBlock();
-            if (m_stateMachine != null) throw new InvalidOperationException(Environment.GetResourceString("AsyncMethodBuilder_InstanceNotInitialized"));
+            if (m_stateMachine != null) ThrowHelper.ThrowInvalidOperationException(ExceptionResource.AsyncMethodBuilder_InstanceNotInitialized);
             m_stateMachine = stateMachine;
         }
 
@@ -1104,9 +1118,9 @@ namespace System.Runtime.CompilerServices
             return action;
         }
 
-    ///<summary>
-    /// Given an action, see if it is a contiunation wrapper and has a Task associated with it.  If so return it (null otherwise)
-    ///</summary>
+        ///<summary>
+        /// Given an action, see if it is a contiunation wrapper and has a Task associated with it.  If so return it (null otherwise)
+        ///</summary>
         internal static Task TryGetContinuationTask(Action action)
         {
             if (action != null) 
index 89ba298..0a67413 100644 (file)
@@ -2337,7 +2337,8 @@ namespace System.Threading.Tasks
 
             public void Invoke(Task completingTask)
             {
-                if (Interlocked.CompareExchange(ref m_firstTaskAlreadyCompleted, 1, 0) == 0)
+                if (m_firstTaskAlreadyCompleted == 0 &&
+                    Interlocked.Exchange(ref m_firstTaskAlreadyCompleted, 1) == 0)
                 {
                     if (AsyncCausalityTracer.LoggingOn)
                     {
index c425bb5..99f074d 100644 (file)
@@ -362,6 +362,7 @@ namespace System {
         text,
         callBack,
         type,
+        stateMachine,
     }
 
     //
@@ -467,6 +468,7 @@ namespace System {
         ConcurrentCollection_SyncRoot_NotSupported,
         ArgumentOutOfRange_Enum,
         InvalidOperation_HandleIsNotInitialized,
+        AsyncMethodBuilder_InstanceNotInitialized,
     }
 }