From 4649ccd1548bbe50963d9a75ac6e82ba189d8e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 26 Apr 2024 14:25:03 +0200 Subject: [PATCH 1/2] Try to reduce cost of Async Contributes to #79204. I don't know if this will be considered mergeable, but I wanted to at least try something. For apps that use async a lot (like the Stage2 app we use for Goldilocks), the async infrastructure can cost 10% of the entire executable. Shuffling a couple things in `GetStateMachineBox` I was able to get 0.23% saving. It's miniscule. In general async is death by a thousand papercuts so I don't see a silver bullet. --- .../AsyncMethodBuilderCore.cs | 6 ++ .../AsyncTaskMethodBuilderT.cs | 88 ++++++++++--------- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs index 459fd26e6abb3..7cb85c290c755 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs @@ -102,6 +102,12 @@ internal static string GetAsyncStateMachineDescription(IAsyncStateMachine stateM return sb.ToString(); } + [MethodImpl(MethodImplOptions.NoInlining)] + internal static void LogTraceOperationBegin(Task t, Type stateMachineType) + { + TplEventSource.Log.TraceOperationBegin(t.Id, "Async: " + stateMachineType.Name, 0); + } + internal static Action CreateContinuationWrapper(Action continuation, Action invokeAction, Task innerTask) => new ContinuationWrapper(continuation, invokeAction, innerTask).Invoke; diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs index c3064ad22114b..ff0c3a43ac45e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs @@ -157,6 +157,8 @@ private static IAsyncStateMachineBox GetStateMachineBox( { ExecutionContext? currentContext = ExecutionContext.Capture(); + IAsyncStateMachineBox result; + // Check first for the most common case: not the first yield in an async method. // In this case, the first yield will have already "boxed" the state machine in // a strongly-typed manner into an AsyncStateMachineBox. It will already contain @@ -168,9 +170,8 @@ private static IAsyncStateMachineBox GetStateMachineBox( { stronglyTypedBox.Context = currentContext; } - return stronglyTypedBox; + result = stronglyTypedBox; } - // The least common case: we have a weakly-typed boxed. This results if the debugger // or some other use of reflection accesses a property like ObjectIdForDebugger or a // method like SetNotificationForWaitCompletion prior to the first await happening. In @@ -180,64 +181,67 @@ private static IAsyncStateMachineBox GetStateMachineBox( // result in a boxing allocation when storing the TStateMachine if it's a struct, but // this only happens in active debugging scenarios where such performance impact doesn't // matter. - if (taskField is AsyncStateMachineBox weaklyTypedBox) + else if (taskField is AsyncStateMachineBox weaklyTypedBox) { // If this is the first await, we won't yet have a state machine, so store it. if (weaklyTypedBox.StateMachine == null) { Debugger.NotifyOfCrossThreadDependency(); // same explanation as with usage below - weaklyTypedBox.StateMachine = stateMachine; + weaklyTypedBox.StateMachine = Unsafe.As(RuntimeHelpers.Box(ref Unsafe.As(ref stateMachine), typeof(TStateMachine).TypeHandle)); } // Update the context. This only happens with a debugger, so no need to spend // extra IL checking for equality before doing the assignment. weaklyTypedBox.Context = currentContext; - return weaklyTypedBox; + result = weaklyTypedBox; } - - // Alert a listening debugger that we can't make forward progress unless it slips threads. - // If we don't do this, and a method that uses "await foo;" is invoked through funceval, - // we could end up hooking up a callback to push forward the async method's state machine, - // the debugger would then abort the funceval after it takes too long, and then continuing - // execution could result in another callback being hooked up. At that point we have - // multiple callbacks registered to push the state machine, which could result in bad behavior. - Debugger.NotifyOfCrossThreadDependency(); - - // At this point, taskField should really be null, in which case we want to create the box. - // However, in a variety of debugger-related (erroneous) situations, it might be non-null, - // e.g. if the Task property is examined in a Watch window, forcing it to be lazily-initialized - // as a Task rather than as an AsyncStateMachineBox. The worst that happens in such - // cases is we lose the ability to properly step in the debugger, as the debugger uses that - // object's identity to track this specific builder/state machine. As such, we proceed to - // overwrite whatever's there anyway, even if it's non-null. + else + { + // Alert a listening debugger that we can't make forward progress unless it slips threads. + // If we don't do this, and a method that uses "await foo;" is invoked through funceval, + // we could end up hooking up a callback to push forward the async method's state machine, + // the debugger would then abort the funceval after it takes too long, and then continuing + // execution could result in another callback being hooked up. At that point we have + // multiple callbacks registered to push the state machine, which could result in bad behavior. + Debugger.NotifyOfCrossThreadDependency(); + + // At this point, taskField should really be null, in which case we want to create the box. + // However, in a variety of debugger-related (erroneous) situations, it might be non-null, + // e.g. if the Task property is examined in a Watch window, forcing it to be lazily-initialized + // as a Task rather than as an AsyncStateMachineBox. The worst that happens in such + // cases is we lose the ability to properly step in the debugger, as the debugger uses that + // object's identity to track this specific builder/state machine. As such, we proceed to + // overwrite whatever's there anyway, even if it's non-null. #if NATIVEAOT - // DebugFinalizableAsyncStateMachineBox looks like a small type, but it actually is not because - // it will have a copy of all the slots from its parent. It will add another hundred(s) bytes - // per each async method in NativeAOT binaries without adding much value. Avoid - // generating this extra code until a better solution is implemented. - var box = new AsyncStateMachineBox(); + // DebugFinalizableAsyncStateMachineBox looks like a small type, but it actually is not because + // it will have a copy of all the slots from its parent. It will add another hundred(s) bytes + // per each async method in NativeAOT binaries without adding much value. Avoid + // generating this extra code until a better solution is implemented. + var box = new AsyncStateMachineBox(); #else - AsyncStateMachineBox box = AsyncMethodBuilderCore.TrackAsyncMethodCompletion ? - CreateDebugFinalizableAsyncStateMachineBox() : - new AsyncStateMachineBox(); + AsyncStateMachineBox box = AsyncMethodBuilderCore.TrackAsyncMethodCompletion ? + CreateDebugFinalizableAsyncStateMachineBox() : + new AsyncStateMachineBox(); #endif - taskField = box; // important: this must be done before storing stateMachine into box.StateMachine! - box.StateMachine = stateMachine; - box.Context = currentContext; + taskField = box; // important: this must be done before storing stateMachine into box.StateMachine! + box.StateMachine = stateMachine; + box.Context = currentContext; - // Log the creation of the state machine box object / task for this async method. - if (TplEventSource.Log.IsEnabled()) - { - TplEventSource.Log.TraceOperationBegin(box.Id, "Async: " + stateMachine.GetType().Name, 0); - } + // Log the creation of the state machine box object / task for this async method. + if (TplEventSource.Log.IsEnabled()) + { + AsyncMethodBuilderCore.LogTraceOperationBegin(box, stateMachine.GetType()); + } - // And if async debugging is enabled, track the task. - if (Threading.Tasks.Task.s_asyncDebuggingEnabled) - { - Threading.Tasks.Task.AddToActiveTasks(box); + // And if async debugging is enabled, track the task. + if (Threading.Tasks.Task.s_asyncDebuggingEnabled) + { + Threading.Tasks.Task.AddToActiveTasks(box); + } + result = box; } - return box; + return result; } #if !NATIVEAOT From 596f68112aeafd73ed8c839bf7ff2b008f3308a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Mon, 29 Apr 2024 08:46:26 +0200 Subject: [PATCH 2/2] Undo manual box --- .../System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs index ff0c3a43ac45e..5a2ccb1d63549 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncTaskMethodBuilderT.cs @@ -187,7 +187,7 @@ private static IAsyncStateMachineBox GetStateMachineBox( if (weaklyTypedBox.StateMachine == null) { Debugger.NotifyOfCrossThreadDependency(); // same explanation as with usage below - weaklyTypedBox.StateMachine = Unsafe.As(RuntimeHelpers.Box(ref Unsafe.As(ref stateMachine), typeof(TStateMachine).TypeHandle)); + weaklyTypedBox.StateMachine = stateMachine; } // Update the context. This only happens with a debugger, so no need to spend