Skip to content

Commit

Permalink
Disable value task pooling for CoreRT for native AOT
Browse files Browse the repository at this point in the history
This is experimental feature that adds about 1.7kB in binary footprint per method returning ValueTask.

Ifdef it out for native AOT until we figure out what to do with it. See dotnet/runtime#13633.
  • Loading branch information
jkotas committed Oct 26, 2020
1 parent a897e5c commit 09dd5f8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
<Nullable>enable</Nullable>

<!-- Ignore all previous constants since SPCL is sensitive to what is defined and the Sdk adds some by default -->
<DefineConstants>CORECLR;NETCOREAPP;SYSTEM_PRIVATE_CORELIB</DefineConstants>
<DefineConstants>CORECLR;NETCOREAPP;SYSTEM_PRIVATE_CORELIB;FEATURE_POOLASYNCVALUETASKS</DefineConstants>
<DisableImplicitConfigurationDefines>true</DisableImplicitConfigurationDefines>

<SkipCommonResourcesIncludes>true</SkipCommonResourcesIncludes>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ internal static class AsyncTaskCache
/// <summary>The maximum value, exclusive, for which we want a cached task.</summary>
internal const int ExclusiveInt32Max = 9;

#if FEATURE_POOLASYNCVALUETASKS
/// <summary>true if we should use reusable boxes for async completions of ValueTask methods; false if we should use tasks.</summary>
/// <remarks>
/// We rely on tiered compilation turning this into a const and doing dead code elimination to make checks on this efficient.
Expand All @@ -40,6 +41,7 @@ private static int GetPoolAsyncValueTasksLimitValue() =>
int.TryParse(Environment.GetEnvironmentVariable("DOTNET_SYSTEM_THREADING_POOLASYNCVALUETASKSLIMIT"), out int result) && result > 0 ?
result :
Environment.ProcessorCount * 4; // arbitrary default value
#endif

/// <summary>Creates a non-disposable task.</summary>
/// <typeparam name="TResult">Specifies the result type.</typeparam>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public struct AsyncValueTaskMethodBuilder
/// <summary>Sentinel object used to indicate that the builder completed synchronously and successfully.</summary>
private static readonly object s_syncSuccessSentinel = AsyncValueTaskMethodBuilder<VoidTaskResult>.s_syncSuccessSentinel;

/// <summary>The wrapped state machine box or task, based on the value of <see cref="AsyncTaskCache.s_valueTaskPoolingEnabled"/>.</summary>
/// <summary>The wrapped state machine box or task, based on the value of s_valueTaskPoolingEnabled.</summary>
/// <remarks>
/// If the operation completed synchronously and successfully, this will be <see cref="s_syncSuccessSentinel"/>.
/// </remarks>
Expand Down Expand Up @@ -46,10 +46,12 @@ public void SetResult()
{
m_task = s_syncSuccessSentinel;
}
#if FEATURE_POOLASYNCVALUETASKS
else if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
Unsafe.As<StateMachineBox>(m_task).SetResult(default);
}
#endif
else
{
AsyncTaskMethodBuilder<VoidTaskResult>.SetExistingTaskResult(Unsafe.As<Task<VoidTaskResult>>(m_task), default);
Expand All @@ -60,11 +62,13 @@ public void SetResult()
/// <param name="exception">The exception to bind to the task.</param>
public void SetException(Exception exception)
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
AsyncValueTaskMethodBuilder<VoidTaskResult>.SetException(exception, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<VoidTaskResult>.SetException(exception, ref Unsafe.As<object?, Task<VoidTaskResult>?>(ref m_task));
}
Expand All @@ -88,6 +92,7 @@ public ValueTask Task
// "work" but in a degraded mode, as we don't know the TStateMachine type here, and thus we use a box around
// the interface instead.

#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
var box = Unsafe.As<StateMachineBox?>(m_task);
Expand All @@ -98,6 +103,7 @@ public ValueTask Task
return new ValueTask(box, box.Version);
}
else
#endif
{
var task = Unsafe.As<Task<VoidTaskResult>?>(m_task);
if (task is null)
Expand All @@ -118,11 +124,13 @@ public void AwaitOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter, ref
where TAwaiter : INotifyCompletion
where TStateMachine : IAsyncStateMachine
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
AsyncValueTaskMethodBuilder<VoidTaskResult>.AwaitOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<VoidTaskResult>.AwaitOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, Task<VoidTaskResult>?>(ref m_task));
}
Expand All @@ -138,11 +146,13 @@ public void AwaitUnsafeOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter
where TAwaiter : ICriticalNotifyCompletion
where TStateMachine : IAsyncStateMachine
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
AsyncValueTaskMethodBuilder<VoidTaskResult>.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<VoidTaskResult>.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, Task<VoidTaskResult>?>(ref m_task));
}
Expand All @@ -162,8 +172,11 @@ internal object ObjectIdForDebugger
{
if (m_task is null)
{
m_task = AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)
m_task =
#if FEATURE_POOLASYNCVALUETASKS
AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)
AsyncValueTaskMethodBuilder<VoidTaskResult>.CreateWeaklyTypedStateMachineBox() :
#endif
AsyncTaskMethodBuilder<VoidTaskResult>.CreateWeaklyTypedStateMachineBox();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ public struct AsyncValueTaskMethodBuilder<TResult>
/// is valid for the mode in which we're operating. As such, it's cached on the generic builder per TResult
/// rather than having one sentinel instance for all types.
/// </remarks>
internal static readonly object s_syncSuccessSentinel = AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)
new SyncSuccessSentinelStateMachineBox() :
internal static readonly object s_syncSuccessSentinel =
#if FEATURE_POOLASYNCVALUETASKS
AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)new SyncSuccessSentinelStateMachineBox() :
#endif
new Task<TResult>(default(TResult)!);

/// <summary>The wrapped state machine or task. If the operation completed synchronously and successfully, this will be a sentinel object compared by reference identity.</summary>
Expand Down Expand Up @@ -56,10 +58,12 @@ public void SetResult(TResult result)
_result = result;
m_task = s_syncSuccessSentinel;
}
#if FEATURE_POOLASYNCVALUETASKS
else if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
Unsafe.As<StateMachineBox>(m_task).SetResult(result);
}
#endif
else
{
AsyncTaskMethodBuilder<TResult>.SetExistingTaskResult(Unsafe.As<Task<TResult>>(m_task), result);
Expand All @@ -70,11 +74,13 @@ public void SetResult(TResult result)
/// <param name="exception">The exception to bind to the value task.</param>
public void SetException(Exception exception)
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
SetException(exception, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<TResult>.SetException(exception, ref Unsafe.As<object?, Task<TResult>?>(ref m_task));
}
Expand Down Expand Up @@ -108,6 +114,7 @@ public ValueTask<TResult> Task
// "work" but in a degraded mode, as we don't know the TStateMachine type here, and thus we use a box around
// the interface instead.

#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
var box = Unsafe.As<StateMachineBox?>(m_task);
Expand All @@ -118,6 +125,7 @@ public ValueTask<TResult> Task
return new ValueTask<TResult>(box, box.Version);
}
else
#endif
{
var task = Unsafe.As<Task<TResult>?>(m_task);
if (task is null)
Expand All @@ -138,11 +146,13 @@ public void AwaitOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter, ref
where TAwaiter : INotifyCompletion
where TStateMachine : IAsyncStateMachine
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
AwaitOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<TResult>.AwaitOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, Task<TResult>?>(ref m_task));
}
Expand Down Expand Up @@ -173,11 +183,13 @@ public void AwaitUnsafeOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter
where TAwaiter : ICriticalNotifyCompletion
where TStateMachine : IAsyncStateMachine
{
#if FEATURE_POOLASYNCVALUETASKS
if (AsyncTaskCache.s_valueTaskPoolingEnabled)
{
AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, StateMachineBox?>(ref m_task));
}
else
#endif
{
AsyncTaskMethodBuilder<TResult>.AwaitUnsafeOnCompleted(ref awaiter, ref stateMachine, ref Unsafe.As<object?, Task<TResult>?>(ref m_task));
}
Expand Down Expand Up @@ -286,8 +298,10 @@ internal object ObjectIdForDebugger
{
if (m_task is null)
{
m_task = AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)
CreateWeaklyTypedStateMachineBox() :
m_task =
#if FEATURE_POOLASYNCVALUETASKS
AsyncTaskCache.s_valueTaskPoolingEnabled ? (object)CreateWeaklyTypedStateMachineBox() :
#endif
AsyncTaskMethodBuilder<TResult>.CreateWeaklyTypedStateMachineBox();
}

Expand Down Expand Up @@ -346,13 +360,16 @@ private sealed class StateMachineBox<TStateMachine> :
{
/// <summary>Delegate used to invoke on an ExecutionContext when passed an instance of this box type.</summary>
private static readonly ContextCallback s_callback = ExecutionContextCallback;

#if FEATURE_POOLASYNCVALUETASKS
/// <summary>Lock used to protected the shared cache of boxes.</summary>
/// <remarks>The code that uses this assumes a runtime without thread aborts.</remarks>
private static int s_cacheLock;
/// <summary>Singly-linked list cache of boxes.</summary>
private static StateMachineBox<TStateMachine>? s_cache;
/// <summary>The number of items stored in <see cref="s_cache"/>.</summary>
private static int s_cacheSize;
#endif

// TODO:
// AsyncTaskMethodBuilder logs about the state machine box lifecycle; AsyncValueTaskMethodBuilder currently
Expand All @@ -370,6 +387,7 @@ private sealed class StateMachineBox<TStateMachine> :
[MethodImpl(MethodImplOptions.AggressiveInlining)] // only one caller
internal static StateMachineBox<TStateMachine> GetOrCreateBox()
{
#if FEATURE_POOLASYNCVALUETASKS
// Try to acquire the lock to access the cache. If there's any contention, don't use the cache.
if (Interlocked.CompareExchange(ref s_cacheLock, 1, 0) == 0)
{
Expand All @@ -393,6 +411,7 @@ internal static StateMachineBox<TStateMachine> GetOrCreateBox()
// Release the lock.
Volatile.Write(ref s_cacheLock, 0);
}
#endif

// Couldn't quickly get a cached instance, so create a new instance.
return new StateMachineBox<TStateMachine>();
Expand All @@ -407,6 +426,7 @@ private void ReturnOrDropBox()
// the caller keeps the box alive for an arbitrary period of time.
ClearStateUponCompletion();

#if FEATURE_POOLASYNCVALUETASKS
// Reset the MRVTSC. We can either do this here, in which case we may be paying the (small) overhead
// to reset the box even if we're going to drop it, or we could do it while holding the lock, in which
// case we'll only reset it if necessary but causing the lock to be held for longer, thereby causing
Expand Down Expand Up @@ -440,6 +460,7 @@ private void ReturnOrDropBox()
// Release the lock.
Volatile.Write(ref s_cacheLock, 0);
}
#endif
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<Nullable>enable</Nullable>

<!-- Ignore all previous constants since SPCL is sensitive to what is defined and the Sdk adds some by default -->
<DefineConstants>MONO;NETCOREAPP;SYSTEM_PRIVATE_CORELIB</DefineConstants>
<DefineConstants>MONO;NETCOREAPP;SYSTEM_PRIVATE_CORELIB;FEATURE_POOLASYNCVALUETASKS</DefineConstants>
<DisableImplicitConfigurationDefines>true</DisableImplicitConfigurationDefines>

<SkipCommonResourcesIncludes>true</SkipCommonResourcesIncludes>
Expand Down

0 comments on commit 09dd5f8

Please sign in to comment.