Remove most RuntimeAsyncTask<T> instance methods#127447
Remove most RuntimeAsyncTask<T> instance methods#127447jakobbotsch wants to merge 1 commit intodotnet:mainfrom
Conversation
Introduce `RuntimeAsyncTaskCore` that contains most of the runtime async machinery, to avoid this machinery being instantiated for every `T` in a `RuntimeAsyncTask<T>`. To do so we need a few changes: - The delegates in `RuntimeAsyncTask<T>` called `DispatchContinuations` directly. Now there are delegates in `RuntimeAsyncTaskCore` that call `ExecuteFromThreadPool`, which is known to delegate to `DispatchContinuations` for `RuntimeAsyncTask<T>`. - `DispatchContinuations` needs to access `Task<T>.m_result`. This now gets passed as a byref to `RuntimeAsyncTaskCore.DispatchContinuations`.
There was a problem hiding this comment.
Pull request overview
This PR refactors the CoreCLR runtime-async “suspend/resume” machinery by moving most per-T logic out of RuntimeAsyncTask<T> into a new non-generic RuntimeAsyncTaskCore, aiming to reduce generic instantiations and associated code size/overhead.
Changes:
- Introduces
RuntimeAsyncTaskCorestatic helpers and routesRuntimeAsyncTask<T>dispatch/suspension logic through it, passingTask<T>.m_resultstorage byref. - Updates completion flow to use
Task.TrySetResult()(implied result is the currentm_resultvalue) instead ofTrySetResult(m_result). - Adjusts a stack reflection test to match the new dispatcher signature.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs |
Extracts runtime-async dispatch/suspension logic into RuntimeAsyncTaskCore and updates dispatch paths/callbacks accordingly. |
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs |
Broadens m_stateObject visibility and updates TrySetResult() remark to align with implied-result behavior. |
src/tests/async/reflection/reflection.cs |
Updates expected stack frame method signature for the dispatcher after refactor. |
| Debug.Assert(state is Task && state.GetType().GetGenericTypeDefinition() == typeof(RuntimeAsyncTask<>)); | ||
| Task runtimeAsyncTask = Unsafe.As<Task>(state); | ||
| // Relies on us knowing that RuntimeAsyncTask.ExecuteFromThreadPool is a simple forward to DispatchContinuations | ||
| runtimeAsyncTask.ExecuteFromThreadPool(null!); | ||
| }; |
There was a problem hiding this comment.
In this path state is passed through SynchronizationContext.Post, which can be overridden by user code and may invoke the callback with an unexpected (or null) state. Using Unsafe.As<Task>(state) can turn that into a type-safety hole; prefer a normal cast/pattern match (and fail fast if it's not the expected RuntimeAsyncTask<>), and avoid relying on ExecuteFromThreadPool(null!) as a dispatch mechanism.
|
Tagging subscribers to this area: @agocke |
Introduce
RuntimeAsyncTaskCorethat contains most of the runtime async machinery, to avoid this machinery being instantiated for everyTin aRuntimeAsyncTask<T>. To do so we need a few changes:RuntimeAsyncTask<T>calledDispatchContinuationsdirectly. Now there are delegates inRuntimeAsyncTaskCorethat callExecuteFromThreadPool, which is known to delegate toDispatchContinuationsforRuntimeAsyncTask<T>.DispatchContinuationsneeds to accessTask<T>.m_result. This now gets passed as a byref toRuntimeAsyncTaskCore.DispatchContinuations.There is a 3-4% perf regression for micro benchmarks suspending/resuming that I am working on.
NativeAOT test sizes