From 3998de60786f1bab3b8421f937afb2ded6eb611a Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 19 Sep 2022 18:06:47 -0400 Subject: [PATCH 1/6] =?UTF-8?q?=EF=BB=BFAdded=20AwaitHelper=20to=20properl?= =?UTF-8?q?y=20wait=20for=20ValueTasks.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Code/DeclarationsProvider.cs | 14 +- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 112 +++++++++++++ .../IlGeneratorStatementExtensions.cs | 31 ++++ .../Templates/BenchmarkType.txt | 4 + .../Emit/Implementation/ConsumableTypeInfo.cs | 33 ++-- .../Emitters/RunnableEmitter.cs | 96 +++++------ .../Runnable/RunnableConstants.cs | 1 + .../BenchmarkActionFactory_Implementations.cs | 9 +- .../Validators/ExecutionValidatorBase.cs | 20 +-- .../AllSetupAndCleanupTest.cs | 156 ++++++++---------- .../AsyncBenchmarksTests.cs | 43 ++++- .../InProcessEmitTest.cs | 14 ++ .../Validators/ExecutionValidatorTests.cs | 78 +++++++++ 13 files changed, 425 insertions(+), 186 deletions(-) create mode 100644 src/BenchmarkDotNet/Helpers/AwaitHelper.cs diff --git a/src/BenchmarkDotNet/Code/DeclarationsProvider.cs b/src/BenchmarkDotNet/Code/DeclarationsProvider.cs index ddf78eb572..21e3ce54c1 100644 --- a/src/BenchmarkDotNet/Code/DeclarationsProvider.cs +++ b/src/BenchmarkDotNet/Code/DeclarationsProvider.cs @@ -63,7 +63,7 @@ private string GetMethodName(MethodInfo method) (method.ReturnType.GetGenericTypeDefinition() == typeof(Task<>) || method.ReturnType.GetGenericTypeDefinition() == typeof(ValueTask<>)))) { - return $"() => {method.Name}().GetAwaiter().GetResult()"; + return $"() => awaitHelper.GetResult({method.Name}())"; } return method.Name; @@ -149,12 +149,10 @@ internal class TaskDeclarationsProvider : VoidDeclarationsProvider { public TaskDeclarationsProvider(Descriptor descriptor) : base(descriptor) { } - // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, - // and will eventually throw actual exception, not aggregated one public override string WorkloadMethodDelegate(string passArguments) - => $"({passArguments}) => {{ {Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult(); }}"; + => $"({passArguments}) => {{ awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; - public override string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult()"; + public override string GetWorkloadMethodCall(string passArguments) => $"awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; protected override Type WorkloadMethodReturnType => typeof(void); } @@ -168,11 +166,9 @@ public GenericTaskDeclarationsProvider(Descriptor descriptor) : base(descriptor) protected override Type WorkloadMethodReturnType => Descriptor.WorkloadMethod.ReturnType.GetTypeInfo().GetGenericArguments().Single(); - // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, - // and will eventually throw actual exception, not aggregated one public override string WorkloadMethodDelegate(string passArguments) - => $"({passArguments}) => {{ return {Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult(); }}"; + => $"({passArguments}) => {{ return awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; - public override string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult()"; + public override string GetWorkloadMethodCall(string passArguments) => $"awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs new file mode 100644 index 0000000000..eb27c4cfa2 --- /dev/null +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -0,0 +1,112 @@ +using System; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +namespace BenchmarkDotNet.Helpers +{ + public class AwaitHelper + { + private readonly object awaiterLock = new object(); + private readonly Action awaiterCallback; + private bool awaiterCompleted; + + public AwaitHelper() + { + awaiterCallback = AwaiterCallback; + } + + private void AwaiterCallback() + { + lock (awaiterLock) + { + awaiterCompleted = true; + System.Threading.Monitor.Pulse(awaiterLock); + } + } + + // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, + // and will eventually throw actual exception, not aggregated one + public void GetResult(Task task) + { + task.GetAwaiter().GetResult(); + } + + public T GetResult(Task task) + { + return task.GetAwaiter().GetResult(); + } + + // It is illegal to call GetResult from an uncomplete ValueTask, so we must hook up a callback. + public void GetResult(ValueTask task) + { + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) + { + lock (awaiterLock) + { + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) + { + System.Threading.Monitor.Wait(awaiterLock); + } + } + } + awaiter.GetResult(); + } + + public T GetResult(ValueTask task) + { + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) + { + lock (awaiterLock) + { + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) + { + System.Threading.Monitor.Wait(awaiterLock); + } + } + } + return awaiter.GetResult(); + } + + internal static MethodInfo GetGetResultMethod(Type taskType) + { + if (!taskType.IsGenericType) + { + return typeof(AwaitHelper).GetMethod(nameof(AwaitHelper.GetResult), BindingFlags.Public | BindingFlags.Instance, null, new Type[1] { taskType }, null); + } + + Type compareType = taskType.GetGenericTypeDefinition() == typeof(ValueTask<>) ? typeof(ValueTask<>) + : typeof(Task).IsAssignableFrom(taskType.GetGenericTypeDefinition()) ? typeof(Task<>) + : null; + if (compareType == null) + { + return null; + } + var resultType = taskType + .GetMethod(nameof(Task.GetAwaiter), BindingFlags.Public | BindingFlags.Instance) + .ReturnType + .GetMethod(nameof(TaskAwaiter.GetResult), BindingFlags.Public | BindingFlags.Instance) + .ReturnType; + return typeof(AwaitHelper).GetMethods(BindingFlags.Public | BindingFlags.Instance) + .First(m => + { + if (m.Name != nameof(AwaitHelper.GetResult)) return false; + Type paramType = m.GetParameters().First().ParameterType; + // We have to compare the types indirectly, == check doesn't work. + return paramType.Assembly == compareType.Assembly && paramType.Namespace == compareType.Namespace && paramType.Name == compareType.Name; + }) + .MakeGenericMethod(new[] { resultType }); + } + } +} diff --git a/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs b/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs index 6d601e6495..65b016a0dd 100644 --- a/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs +++ b/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs @@ -42,6 +42,37 @@ public static void EmitVoidReturn(this ILGenerator ilBuilder, MethodBuilder meth ilBuilder.Emit(OpCodes.Ret); } + public static void EmitSetFieldToNewInstance( + this ILGenerator ilBuilder, + FieldBuilder field, + Type instanceType) + { + if (field.IsStatic) + throw new ArgumentException("The field should be instance field", nameof(field)); + + if (instanceType != null) + { + /* + IL_0006: ldarg.0 + IL_0007: newobj instance void BenchmarkDotNet.Helpers.AwaitHelper::.ctor() + IL_000c: stfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Autogenerated.Runnable_0::awaitHelper + */ + var ctor = instanceType.GetConstructor(Array.Empty()); + if (ctor == null) + throw new InvalidOperationException($"Bug: instanceType {instanceType.Name} does not have a 0-parameter accessible constructor."); + + ilBuilder.Emit(OpCodes.Ldarg_0); + ilBuilder.Emit(OpCodes.Newobj, ctor); + ilBuilder.Emit(OpCodes.Stfld, field); + } + else + { + ilBuilder.Emit(OpCodes.Ldarg_0); + ilBuilder.Emit(OpCodes.Ldnull); + ilBuilder.Emit(OpCodes.Stfld, field); + } + } + public static void EmitSetDelegateToThisField( this ILGenerator ilBuilder, FieldBuilder delegateField, diff --git a/src/BenchmarkDotNet/Templates/BenchmarkType.txt b/src/BenchmarkDotNet/Templates/BenchmarkType.txt index f17737d646..4bff1cdd98 100644 --- a/src/BenchmarkDotNet/Templates/BenchmarkType.txt +++ b/src/BenchmarkDotNet/Templates/BenchmarkType.txt @@ -57,6 +57,8 @@ public Runnable_$ID$() { + awaitHelper = new BenchmarkDotNet.Helpers.AwaitHelper(); + globalSetupAction = $GlobalSetupMethodName$; globalCleanupAction = $GlobalCleanupMethodName$; iterationSetupAction = $IterationSetupMethodName$; @@ -66,6 +68,8 @@ $InitializeArgumentFields$ } + private readonly BenchmarkDotNet.Helpers.AwaitHelper awaitHelper; + private System.Action globalSetupAction; private System.Action globalCleanupAction; private System.Action iterationSetupAction; diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs index f8d344b1c2..147a514058 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs @@ -1,5 +1,7 @@ using BenchmarkDotNet.Engines; using System; +using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Threading.Tasks; @@ -16,28 +18,24 @@ public ConsumableTypeInfo(Type methodReturnType) OriginMethodReturnType = methodReturnType; - // Please note this code does not support await over extension methods. - var getAwaiterMethod = methodReturnType.GetMethod(nameof(Task.GetAwaiter), BindingFlagsPublicInstance); - if (getAwaiterMethod == null) + // Only support (Value)Task for parity with other toolchains (and so we can use AwaitHelper). + IsAwaitable = methodReturnType == typeof(Task) || methodReturnType == typeof(ValueTask) + || (methodReturnType.GetTypeInfo().IsGenericType + && (methodReturnType.GetTypeInfo().GetGenericTypeDefinition() == typeof(Task<>) + || methodReturnType.GetTypeInfo().GetGenericTypeDefinition() == typeof(ValueTask<>))); + + if (!IsAwaitable) { WorkloadMethodReturnType = methodReturnType; } else { - var getResultMethod = getAwaiterMethod + WorkloadMethodReturnType = methodReturnType + .GetMethod(nameof(Task.GetAwaiter), BindingFlagsPublicInstance) .ReturnType - .GetMethod(nameof(TaskAwaiter.GetResult), BindingFlagsPublicInstance); - - if (getResultMethod == null) - { - WorkloadMethodReturnType = methodReturnType; - } - else - { - WorkloadMethodReturnType = getResultMethod.ReturnType; - GetAwaiterMethod = getAwaiterMethod; - GetResultMethod = getResultMethod; - } + .GetMethod(nameof(TaskAwaiter.GetResult), BindingFlagsPublicInstance) + .ReturnType; + GetResultMethod = Helpers.AwaitHelper.GetGetResultMethod(methodReturnType); } if (WorkloadMethodReturnType == null) @@ -74,7 +72,6 @@ public ConsumableTypeInfo(Type methodReturnType) public Type WorkloadMethodReturnType { get; } public Type OverheadMethodReturnType { get; } - public MethodInfo? GetAwaiterMethod { get; } public MethodInfo? GetResultMethod { get; } public bool IsVoid { get; } @@ -82,6 +79,6 @@ public ConsumableTypeInfo(Type methodReturnType) public bool IsConsumable { get; } public FieldInfo? WorkloadConsumableField { get; } - public bool IsAwaitable => GetAwaiterMethod != null && GetResultMethod != null; + public bool IsAwaitable { get; } } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs index 9048474329..12e4072d0b 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs @@ -248,6 +248,7 @@ private static void EmitNoArgsMethodCallPopReturn( private ConsumableTypeInfo consumableInfo; private ConsumeEmitter consumeEmitter; + private FieldBuilder awaitHelperField; private FieldBuilder globalSetupActionField; private FieldBuilder globalCleanupActionField; private FieldBuilder iterationSetupActionField; @@ -413,6 +414,8 @@ private Type EmitWorkloadDelegateType() private void DefineFields() { + awaitHelperField = + runnableBuilder.DefineField(AwaitHelperFieldName, typeof(Helpers.AwaitHelper), FieldAttributes.Private | FieldAttributes.InitOnly); globalSetupActionField = runnableBuilder.DefineField(GlobalSetupActionFieldName, typeof(Action), FieldAttributes.Private); globalCleanupActionField = @@ -434,7 +437,7 @@ private void DefineFields() Type argLocalsType; Type argFieldType; - MethodInfo? opConversion = null; + MethodInfo opConversion = null; if (parameterType.IsByRef) { argLocalsType = parameterType; @@ -582,42 +585,35 @@ private MethodInfo EmitWorkloadImplementation(string methodName) workloadInvokeMethod.ReturnParameter, args); args = methodBuilder.GetEmitParameters(args); - var callResultType = consumableInfo.OriginMethodReturnType; - var awaiterType = consumableInfo.GetAwaiterMethod?.ReturnType - ?? throw new InvalidOperationException($"Bug: {nameof(consumableInfo.GetAwaiterMethod)} is null"); var ilBuilder = methodBuilder.GetILGenerator(); /* - .locals init ( - [0] valuetype [mscorlib]System.Runtime.CompilerServices.TaskAwaiter`1 - ) - */ - var callResultLocal = - ilBuilder.DeclareOptionalLocalForInstanceCall(callResultType, consumableInfo.GetAwaiterMethod); - var awaiterLocal = - ilBuilder.DeclareOptionalLocalForInstanceCall(awaiterType, consumableInfo.GetResultMethod); + IL_0007: ldarg.0 + IL_0008: ldfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Helpers.Runnable_0::awaitHelper + */ + ilBuilder.Emit(OpCodes.Ldarg_0); + ilBuilder.Emit(OpCodes.Ldfld, awaitHelperField); /* - // return TaskSample(arg0). ... ; - IL_0000: ldarg.0 - IL_0001: ldarg.1 - IL_0002: call instance class [mscorlib]System.Threading.Tasks.Task`1 [BenchmarkDotNet]BenchmarkDotNet.Samples.SampleBenchmark::TaskSample(int64) - */ + IL_0026: ldarg.0 + IL_0027: ldloc.0 + IL_0028: ldloc.1 + IL_0029: ldloc.2 + IL_002a: ldloc.3 + IL_002b: call instance class [System.Private.CoreLib]System.Threading.Tasks.Task`1 BenchmarkDotNet.Helpers.Runnable_0::WorkloadMethod(string, string, string, string) + */ if (!Descriptor.WorkloadMethod.IsStatic) ilBuilder.Emit(OpCodes.Ldarg_0); ilBuilder.EmitLdargs(args); ilBuilder.Emit(OpCodes.Call, Descriptor.WorkloadMethod); /* - // ... .GetAwaiter().GetResult(); - IL_0007: callvirt instance valuetype [mscorlib]System.Runtime.CompilerServices.TaskAwaiter`1 class [mscorlib]System.Threading.Tasks.Task`1::GetAwaiter() - IL_000c: stloc.0 - IL_000d: ldloca.s 0 - IL_000f: call instance !0 valuetype [mscorlib]System.Runtime.CompilerServices.TaskAwaiter`1::GetResult() - */ - ilBuilder.EmitInstanceCallThisValueOnStack(callResultLocal, consumableInfo.GetAwaiterMethod); - ilBuilder.EmitInstanceCallThisValueOnStack(awaiterLocal, consumableInfo.GetResultMethod); + // awaitHelper.GetResult(...); + IL_000e: callvirt instance void BenchmarkDotNet.Helpers.AwaitHelper::GetResult(class [System.Private.CoreLib]System.Threading.Tasks.Task) + */ + + ilBuilder.Emit(OpCodes.Callvirt, consumableInfo.GetResultMethod); /* IL_0014: ret @@ -833,19 +829,6 @@ .locals init ( var skipFirstArg = workloadMethod.IsStatic; var argLocals = EmitDeclareArgLocals(ilBuilder, skipFirstArg); - LocalBuilder? callResultLocal = null; - LocalBuilder? awaiterLocal = null; - if (consumableInfo.IsAwaitable) - { - var callResultType = consumableInfo.OriginMethodReturnType; - var awaiterType = consumableInfo.GetAwaiterMethod?.ReturnType - ?? throw new InvalidOperationException($"Bug: {nameof(consumableInfo.GetAwaiterMethod)} is null"); - callResultLocal = - ilBuilder.DeclareOptionalLocalForInstanceCall(callResultType, consumableInfo.GetAwaiterMethod); - awaiterLocal = - ilBuilder.DeclareOptionalLocalForInstanceCall(awaiterType, consumableInfo.GetResultMethod); - } - consumeEmitter.DeclareDisassemblyDiagnoserLocals(ilBuilder); var notElevenLabel = ilBuilder.DefineLabel(); @@ -869,30 +852,38 @@ .locals init ( */ EmitLoadArgFieldsToLocals(ilBuilder, argLocals, skipFirstArg); + if (consumableInfo.IsAwaitable) + { + /* + IL_0026: ldarg.0 + IL_0027: ldfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Helpers.Runnable_0::awaitHelper + */ + ilBuilder.Emit(OpCodes.Ldarg_0); + ilBuilder.Emit(OpCodes.Ldfld, awaitHelperField); + } + /* - // return TaskSample(_argField) ... ; - IL_0011: ldarg.0 - IL_0012: ldloc.0 - IL_0013: call instance class [mscorlib]System.Threading.Tasks.Task`1 [BenchmarkDotNet]BenchmarkDotNet.Samples.SampleBenchmark::TaskSample(int64) - IL_0018: ret + IL_0026: ldarg.0 + IL_0027: ldloc.0 + IL_0028: ldloc.1 + IL_0029: ldloc.2 + IL_002a: ldloc.3 + IL_002b: call instance class [System.Private.CoreLib]System.Threading.Tasks.Task`1 BenchmarkDotNet.Helpers.Runnable_0::WorkloadMethod(string, string, string, string) */ - if (!workloadMethod.IsStatic) + { ilBuilder.Emit(OpCodes.Ldarg_0); + } ilBuilder.EmitLdLocals(argLocals); ilBuilder.Emit(OpCodes.Call, workloadMethod); if (consumableInfo.IsAwaitable) { /* - // ... .GetAwaiter().GetResult(); - IL_0007: callvirt instance valuetype [mscorlib]System.Runtime.CompilerServices.TaskAwaiter`1 class [mscorlib]System.Threading.Tasks.Task`1::GetAwaiter() - IL_000c: stloc.0 - IL_000d: ldloca.s 0 - IL_000f: call instance !0 valuetype [mscorlib]System.Runtime.CompilerServices.TaskAwaiter`1::GetResult() - */ - ilBuilder.EmitInstanceCallThisValueOnStack(callResultLocal, consumableInfo.GetAwaiterMethod); - ilBuilder.EmitInstanceCallThisValueOnStack(awaiterLocal, consumableInfo.GetResultMethod); + // awaitHelper.GetResult(...); + IL_0036: callvirt instance void BenchmarkDotNet.Helpers.AwaitHelper::GetResult(class [System.Private.CoreLib]System.Threading.Tasks.Task) + */ + ilBuilder.Emit(OpCodes.Callvirt, consumableInfo.GetResultMethod); } /* @@ -955,6 +946,7 @@ private void EmitCtorBody() consumeEmitter.OnEmitCtorBody(ctorMethod, ilBuilder); + ilBuilder.EmitSetFieldToNewInstance(awaitHelperField, typeof(Helpers.AwaitHelper)); ilBuilder.EmitSetDelegateToThisField(globalSetupActionField, globalSetupMethod); ilBuilder.EmitSetDelegateToThisField(globalCleanupActionField, globalCleanupMethod); ilBuilder.EmitSetDelegateToThisField(iterationSetupActionField, iterationSetupMethod); diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs index c6e8cd8ae1..920fabb03e 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs @@ -16,6 +16,7 @@ public class RunnableConstants public const string ArgFieldPrefix = "__argField"; public const string ArgParamPrefix = "arg"; + public const string AwaitHelperFieldName = "awaitHelper"; public const string GlobalSetupActionFieldName = "globalSetupAction"; public const string GlobalCleanupActionFieldName = "globalCleanupAction"; public const string IterationSetupActionFieldName = "iterationSetupAction"; diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs index 551e3001c9..fda7d0eef5 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs @@ -89,6 +89,7 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionTask : BenchmarkActionBase { + private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func startTaskCallback; private readonly Action callback; private readonly Action unrolledCallback; @@ -118,7 +119,7 @@ public BenchmarkActionTask(object instance, MethodInfo method, int unrollFactor) private void Overhead() { } // must be kept in sync with TaskDeclarationsProvider.TargetMethodDelegate - private void ExecuteBlocking() => startTaskCallback.Invoke().GetAwaiter().GetResult(); + private void ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); [MethodImpl(CodeGenHelper.AggressiveOptimizationOption)] private void WorkloadActionUnroll(long repeatCount) @@ -137,6 +138,7 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionTask : BenchmarkActionBase { + private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func> startTaskCallback; private readonly Func callback; private readonly Func unrolledCallback; @@ -165,7 +167,7 @@ public BenchmarkActionTask(object instance, MethodInfo method, int unrollFactor) private T Overhead() => default; // must be kept in sync with GenericTaskDeclarationsProvider.TargetMethodDelegate - private T ExecuteBlocking() => startTaskCallback().GetAwaiter().GetResult(); + private T ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); private void InvokeSingleHardcoded() => result = callback(); @@ -188,6 +190,7 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionValueTask : BenchmarkActionBase { + private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func> startTaskCallback; private readonly Func callback; private readonly Func unrolledCallback; @@ -217,7 +220,7 @@ public BenchmarkActionValueTask(object instance, MethodInfo method, int unrollFa private T Overhead() => default; // must be kept in sync with GenericTaskDeclarationsProvider.TargetMethodDelegate - private T ExecuteBlocking() => startTaskCallback().GetAwaiter().GetResult(); + private T ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); private void InvokeSingleHardcoded() => result = callback(); diff --git a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs index 11af691440..9eb40bad88 100644 --- a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs +++ b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs @@ -5,12 +5,15 @@ using System.Threading.Tasks; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Extensions; +using BenchmarkDotNet.Helpers; using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Validators { public abstract class ExecutionValidatorBase : IValidator { + protected AwaitHelper awaitHelper = new (); + protected ExecutionValidatorBase(bool failOnError) { TreatsWarningsAsErrors = failOnError; @@ -130,21 +133,8 @@ private void TryToGetTaskResult(object result) return; } - var returnType = result.GetType(); - if (returnType.IsGenericType && returnType.GetGenericTypeDefinition() == typeof(ValueTask<>)) - { - var asTaskMethod = result.GetType().GetMethod("AsTask"); - result = asTaskMethod.Invoke(result, null); - } - - if (result is Task task) - { - task.GetAwaiter().GetResult(); - } - else if (result is ValueTask valueTask) - { - valueTask.GetAwaiter().GetResult(); - } + AwaitHelper.GetGetResultMethod(result.GetType()) + ?.Invoke(awaitHelper, new[] { result }); } private bool TryToSetParamsFields(object benchmarkTypeInstance, List errors) diff --git a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs index 60bdd52845..459c23c2c2 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs @@ -48,13 +48,20 @@ public AllSetupAndCleanupTest(ITestOutputHelper output) : base(output) { } private static string[] GetActualLogLines(Summary summary) => GetSingleStandardOutput(summary).Where(line => line.StartsWith(Prefix)).ToArray(); - [Fact] - public void AllSetupAndCleanupMethodRunsTest() + [Theory] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarks))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksTask))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksGenericTask))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksValueTask))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksGenericValueTask))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksValueTaskSource))] + [InlineData(typeof(AllSetupAndCleanupAttributeBenchmarksGenericValueTaskSource))] + public void AllSetupAndCleanupMethodRunsTest(Type benchmarkType) { var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); var config = CreateSimpleConfig(job: miniJob); - var summary = CanExecute(config); + var summary = CanExecute(benchmarkType, config); var actualLogLines = GetActualLogLines(summary); foreach (string line in actualLogLines) @@ -83,21 +90,7 @@ public class AllSetupAndCleanupAttributeBenchmarks public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - [Fact] - public void AllSetupAndCleanupMethodRunsAsyncTest() - { - var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(job: miniJob); - - var summary = CanExecute(config); - - var actualLogLines = GetActualLogLines(summary); - foreach (string line in actualLogLines) - Output.WriteLine(line); - Assert.Equal(expectedLogLines, actualLogLines); - } - - public class AllSetupAndCleanupAttributeBenchmarksAsync + public class AllSetupAndCleanupAttributeBenchmarksTask { private int setupCounter; private int cleanupCounter; @@ -115,24 +108,10 @@ public class AllSetupAndCleanupAttributeBenchmarksAsync public Task GlobalCleanup() => Console.Out.WriteLineAsync(GlobalCleanupCalled); [Benchmark] - public Task Benchmark() => Console.Out.WriteLineAsync(BenchmarkCalled); - } - - [Fact] - public void AllSetupAndCleanupMethodRunsAsyncTaskSetupTest() - { - var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(job: miniJob); - - var summary = CanExecute(config); - - var actualLogLines = GetActualLogLines(summary); - foreach (string line in actualLogLines) - Output.WriteLine(line); - Assert.Equal(expectedLogLines, actualLogLines); + public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - public class AllSetupAndCleanupAttributeBenchmarksAsyncTaskSetup + public class AllSetupAndCleanupAttributeBenchmarksGenericTask { private int setupCounter; private int cleanupCounter; @@ -144,30 +123,47 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncTaskSetup public void IterationCleanup() => Console.WriteLine(IterationCleanupCalled + " (" + ++cleanupCounter + ")"); [GlobalSetup] - public Task GlobalSetup() => Console.Out.WriteLineAsync(GlobalSetupCalled); + public async Task GlobalSetup() + { + await Console.Out.WriteLineAsync(GlobalSetupCalled); + + return 42; + } [GlobalCleanup] - public Task GlobalCleanup() => Console.Out.WriteLineAsync(GlobalCleanupCalled); + public async Task GlobalCleanup() + { + await Console.Out.WriteLineAsync(GlobalCleanupCalled); + + return 42; + } [Benchmark] public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - [Fact] - public void AllSetupAndCleanupMethodRunsAsyncGenericTaskSetupTest() + public class AllSetupAndCleanupAttributeBenchmarksValueTask { - var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(job: miniJob); + private int setupCounter; + private int cleanupCounter; - var summary = CanExecute(config); + [IterationSetup] + public void IterationSetup() => Console.WriteLine(IterationSetupCalled + " (" + ++setupCounter + ")"); - var actualLogLines = GetActualLogLines(summary); - foreach (string line in actualLogLines) - Output.WriteLine(line); - Assert.Equal(expectedLogLines, actualLogLines); + [IterationCleanup] + public void IterationCleanup() => Console.WriteLine(IterationCleanupCalled + " (" + ++cleanupCounter + ")"); + + [GlobalSetup] + public ValueTask GlobalSetup() => new ValueTask(Console.Out.WriteLineAsync(GlobalSetupCalled)); + + [GlobalCleanup] + public ValueTask GlobalCleanup() => new ValueTask(Console.Out.WriteLineAsync(GlobalCleanupCalled)); + + [Benchmark] + public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - public class AllSetupAndCleanupAttributeBenchmarksAsyncGenericTaskSetup + public class AllSetupAndCleanupAttributeBenchmarksGenericValueTask { private int setupCounter; private int cleanupCounter; @@ -179,7 +175,7 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncGenericTaskSetup public void IterationCleanup() => Console.WriteLine(IterationCleanupCalled + " (" + ++cleanupCounter + ")"); [GlobalSetup] - public async Task GlobalSetup() + public async ValueTask GlobalSetup() { await Console.Out.WriteLineAsync(GlobalSetupCalled); @@ -187,7 +183,7 @@ public async Task GlobalSetup() } [GlobalCleanup] - public async Task GlobalCleanup() + public async ValueTask GlobalCleanup() { await Console.Out.WriteLineAsync(GlobalCleanupCalled); @@ -198,22 +194,9 @@ public async Task GlobalCleanup() public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - [Fact] - public void AllSetupAndCleanupMethodRunsAsyncValueTaskSetupTest() - { - var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(job: miniJob); - - var summary = CanExecute(config); - - var actualLogLines = GetActualLogLines(summary); - foreach (string line in actualLogLines) - Output.WriteLine(line); - Assert.Equal(expectedLogLines, actualLogLines); - } - - public class AllSetupAndCleanupAttributeBenchmarksAsyncValueTaskSetup + public class AllSetupAndCleanupAttributeBenchmarksValueTaskSource { + private readonly ValueTaskSource valueTaskSource = new (); private int setupCounter; private int cleanupCounter; @@ -224,31 +207,28 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncValueTaskSetup public void IterationCleanup() => Console.WriteLine(IterationCleanupCalled + " (" + ++cleanupCounter + ")"); [GlobalSetup] - public ValueTask GlobalSetup() => new ValueTask(Console.Out.WriteLineAsync(GlobalSetupCalled)); + public ValueTask GlobalSetup() + { + valueTaskSource.Reset(); + Console.Out.WriteLineAsync(GlobalSetupCalled).ContinueWith(_ => valueTaskSource.SetResult(42)); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } [GlobalCleanup] - public ValueTask GlobalCleanup() => new ValueTask(Console.Out.WriteLineAsync(GlobalCleanupCalled)); + public ValueTask GlobalCleanup() + { + valueTaskSource.Reset(); + Console.Out.WriteLineAsync(GlobalCleanupCalled).ContinueWith(_ => valueTaskSource.SetResult(42)); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } [Benchmark] public void Benchmark() => Console.WriteLine(BenchmarkCalled); } - [FactEnvSpecific(EnvRequirement.NonWindows)] - public void AllSetupAndCleanupMethodRunsAsyncGenericValueTaskSetupTest() - { - var miniJob = Job.Default.WithStrategy(RunStrategy.Monitoring).WithWarmupCount(2).WithIterationCount(3).WithInvocationCount(1).WithUnrollFactor(1).WithId("MiniJob"); - var config = CreateSimpleConfig(job: miniJob); - - var summary = CanExecute(config); - - var actualLogLines = GetActualLogLines(summary); - foreach (string line in actualLogLines) - Output.WriteLine(line); - Assert.Equal(expectedLogLines, actualLogLines); - } - - public class AllSetupAndCleanupAttributeBenchmarksAsyncGenericValueTaskSetup + public class AllSetupAndCleanupAttributeBenchmarksGenericValueTaskSource { + private readonly ValueTaskSource valueTaskSource = new (); private int setupCounter; private int cleanupCounter; @@ -259,19 +239,19 @@ public class AllSetupAndCleanupAttributeBenchmarksAsyncGenericValueTaskSetup public void IterationCleanup() => Console.WriteLine(IterationCleanupCalled + " (" + ++cleanupCounter + ")"); [GlobalSetup] - public async ValueTask GlobalSetup() + public ValueTask GlobalSetup() { - await Console.Out.WriteLineAsync(GlobalSetupCalled); - - return 42; + valueTaskSource.Reset(); + Console.Out.WriteLineAsync(GlobalSetupCalled).ContinueWith(_ => valueTaskSource.SetResult(42)); + return new ValueTask(valueTaskSource, valueTaskSource.Token); } [GlobalCleanup] - public async ValueTask GlobalCleanup() + public ValueTask GlobalCleanup() { - await Console.Out.WriteLineAsync(GlobalCleanupCalled); - - return 42; + valueTaskSource.Reset(); + Console.Out.WriteLineAsync(GlobalCleanupCalled).ContinueWith(_ => valueTaskSource.SetResult(42)); + return new ValueTask(valueTaskSource, valueTaskSource.Token); } [Benchmark] diff --git a/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs b/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs index 8eb149b5dc..e9d1f3785e 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs @@ -1,10 +1,27 @@ -using System.Threading.Tasks; +using System; +using System.Threading.Tasks; +using System.Threading.Tasks.Sources; using BenchmarkDotNet.Attributes; using Xunit; using Xunit.Abstractions; namespace BenchmarkDotNet.IntegrationTests { + internal class ValueTaskSource : IValueTaskSource, IValueTaskSource + { + private ManualResetValueTaskSourceCore _core; + + T IValueTaskSource.GetResult(short token) => _core.GetResult(token); + void IValueTaskSource.GetResult(short token) => _core.GetResult(token); + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _core.GetStatus(token); + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _core.GetStatus(token); + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + public void Reset() => _core.Reset(); + public short Token => _core.Version; + public void SetResult(T result) => _core.SetResult(result); + } + public class AsyncBenchmarksTests : BenchmarkTestExecutor { public AsyncBenchmarksTests(ITestOutputHelper output) : base(output) { } @@ -26,6 +43,8 @@ public void TaskReturningMethodsAreAwaited() public class TaskDelayMethods { + private readonly ValueTaskSource valueTaskSource = new (); + private const int MillisecondsDelay = 100; internal const double NanosecondsDelay = MillisecondsDelay * 1e+6; @@ -39,6 +58,17 @@ public class TaskDelayMethods [Benchmark] public ValueTask ReturningValueTask() => new ValueTask(Task.Delay(MillisecondsDelay)); + [Benchmark] + public ValueTask ReturningValueTaskBackByIValueTaskSource() + { + valueTaskSource.Reset(); + Task.Delay(MillisecondsDelay).ContinueWith(_ => + { + valueTaskSource.SetResult(default); + }); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } + [Benchmark] public async Task Awaiting() => await Task.Delay(MillisecondsDelay); @@ -47,6 +77,17 @@ public class TaskDelayMethods [Benchmark] public ValueTask ReturningGenericValueTask() => new ValueTask(ReturningGenericTask()); + + [Benchmark] + public ValueTask ReturningGenericValueTaskBackByIValueTaskSource() + { + valueTaskSource.Reset(); + Task.Delay(MillisecondsDelay).ContinueWith(_ => + { + valueTaskSource.SetResult(default); + }); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } } } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.IntegrationTests/InProcessEmitTest.cs b/tests/BenchmarkDotNet.IntegrationTests/InProcessEmitTest.cs index cb034920a1..a48315c2ef 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/InProcessEmitTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/InProcessEmitTest.cs @@ -153,6 +153,13 @@ public async Task InvokeOnceTaskAsync() Interlocked.Increment(ref Counter); } + [Benchmark] + public async ValueTask InvokeOnceValueTaskAsync() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + [Benchmark] public string InvokeOnceRefType() { @@ -195,6 +202,13 @@ public static async Task InvokeOnceStaticTaskAsync() Interlocked.Increment(ref Counter); } + [Benchmark] + public static async ValueTask InvokeOnceStaticValueTaskAsync() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + [Benchmark] public static string InvokeOnceStaticRefType() { diff --git a/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs index 343bdc53dc..fece3b3939 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Threading.Tasks; +using System.Threading.Tasks.Sources; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Running; using BenchmarkDotNet.Validators; @@ -563,5 +564,82 @@ public async ValueTask GlobalCleanup() [Benchmark] public void NonThrowing() { } } + + private class ValueTaskSource : IValueTaskSource, IValueTaskSource + { + private ManualResetValueTaskSourceCore _core; + + T IValueTaskSource.GetResult(short token) => _core.GetResult(token); + void IValueTaskSource.GetResult(short token) => _core.GetResult(token); + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _core.GetStatus(token); + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _core.GetStatus(token); + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + public void Reset() => _core.Reset(); + public short Token => _core.Version; + public void SetResult(T result) => _core.SetResult(result); + } + + [Fact] + public void AsyncValueTaskBackedByIValueTaskSourceIsAwaitedProperly() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(AsyncValueTaskSource))).ToList(); + + Assert.True(AsyncValueTaskSource.WasCalled); + Assert.Empty(validationErrors); + } + + public class AsyncValueTaskSource + { + private readonly ValueTaskSource valueTaskSource = new (); + + public static bool WasCalled; + + [GlobalSetup] + public ValueTask GlobalSetup() + { + valueTaskSource.Reset(); + Task.Delay(1).ContinueWith(_ => + { + WasCalled = true; + valueTaskSource.SetResult(true); + }); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } + + [Benchmark] + public void NonThrowing() { } + } + + [Fact] + public void AsyncGenericValueTaskBackedByIValueTaskSourceIsAwaitedProperly() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(AsyncGenericValueTaskSource))).ToList(); + + Assert.True(AsyncGenericValueTaskSource.WasCalled); + Assert.Empty(validationErrors); + } + + public class AsyncGenericValueTaskSource + { + private readonly ValueTaskSource valueTaskSource = new (); + + public static bool WasCalled; + + [GlobalSetup] + public ValueTask GlobalSetup() + { + valueTaskSource.Reset(); + Task.Delay(1).ContinueWith(_ => + { + WasCalled = true; + valueTaskSource.SetResult(1); + }); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } + + [Benchmark] + public void NonThrowing() { } + } } } \ No newline at end of file From fb883796d20ae1cfe4434f34d0c87d2ec64b8f1d Mon Sep 17 00:00:00 2001 From: Tim Date: Sun, 25 Sep 2022 15:05:01 -0400 Subject: [PATCH 2/6] Adjust `AwaitHelper` to allow multiple threads to use it concurrently. --- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 116 ++++++++++++--------- 1 file changed, 67 insertions(+), 49 deletions(-) diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs index eb27c4cfa2..85b9829b18 100644 --- a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -8,77 +8,95 @@ namespace BenchmarkDotNet.Helpers { public class AwaitHelper { - private readonly object awaiterLock = new object(); - private readonly Action awaiterCallback; - private bool awaiterCompleted; - - public AwaitHelper() + private class ValueTaskWaiter { - awaiterCallback = AwaiterCallback; - } + private readonly Action awaiterCallback; + private bool awaiterCompleted; - private void AwaiterCallback() - { - lock (awaiterLock) + internal ValueTaskWaiter() { - awaiterCompleted = true; - System.Threading.Monitor.Pulse(awaiterLock); + awaiterCallback = AwaiterCallback; } - } - // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, - // and will eventually throw actual exception, not aggregated one - public void GetResult(Task task) - { - task.GetAwaiter().GetResult(); - } + private void AwaiterCallback() + { + lock (this) + { + awaiterCompleted = true; + System.Threading.Monitor.Pulse(this); + } + } - public T GetResult(Task task) - { - return task.GetAwaiter().GetResult(); - } + // Hook up a callback instead of converting to Task to prevent extra allocations on each benchmark run. + internal void GetResult(ValueTask task) + { + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) + { + lock (this) + { + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) + { + System.Threading.Monitor.Wait(this); + } + } + } + awaiter.GetResult(); + } - // It is illegal to call GetResult from an uncomplete ValueTask, so we must hook up a callback. - public void GetResult(ValueTask task) - { - // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. - var awaiter = task.ConfigureAwait(false).GetAwaiter(); - if (!awaiter.IsCompleted) + internal T GetResult(ValueTask task) { - lock (awaiterLock) + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) + lock (this) { - System.Threading.Monitor.Wait(awaiterLock); + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) + { + System.Threading.Monitor.Wait(this); + } } } + return awaiter.GetResult(); } - awaiter.GetResult(); } - public T GetResult(ValueTask task) + // We use thread static field so that multiple threads can use individual lock object and callback. + [ThreadStatic] + private static ValueTaskWaiter ts_valueTaskWaiter; + + private ValueTaskWaiter CurrentValueTaskWaiter { - // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. - var awaiter = task.ConfigureAwait(false).GetAwaiter(); - if (!awaiter.IsCompleted) + get { - lock (awaiterLock) + if (ts_valueTaskWaiter == null) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) - { - System.Threading.Monitor.Wait(awaiterLock); - } + ts_valueTaskWaiter = new ValueTaskWaiter(); } + return ts_valueTaskWaiter; } - return awaiter.GetResult(); } + // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, + // and will eventually throw actual exception, not aggregated one + public void GetResult(Task task) => task.GetAwaiter().GetResult(); + + public T GetResult(Task task) => task.GetAwaiter().GetResult(); + + // ValueTask can be backed by an IValueTaskSource that only supports asynchronous awaits, so we have to hook up a callback instead of calling .GetAwaiter().GetResult() like we do for Task. + // The alternative is to convert it to Task using .AsTask(), but that causes allocations which we must avoid for memory diagnoser. + public void GetResult(ValueTask task) => CurrentValueTaskWaiter.GetResult(task); + + public T GetResult(ValueTask task) => CurrentValueTaskWaiter.GetResult(task); + internal static MethodInfo GetGetResultMethod(Type taskType) { if (!taskType.IsGenericType) From df776b1ba978e97e5fc047adf7e761908e454eec Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 26 Sep 2022 06:12:00 -0400 Subject: [PATCH 3/6] Changed AwaitHelper to static. --- .../Code/DeclarationsProvider.cs | 10 +- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 95 +++++++++---------- .../IlGeneratorStatementExtensions.cs | 31 ------ .../Templates/BenchmarkType.txt | 4 - .../Emitters/RunnableEmitter.cs | 33 ++----- .../Runnable/RunnableConstants.cs | 1 - .../BenchmarkActionFactory_Implementations.cs | 9 +- .../Validators/ExecutionValidatorBase.cs | 4 +- 8 files changed, 60 insertions(+), 127 deletions(-) diff --git a/src/BenchmarkDotNet/Code/DeclarationsProvider.cs b/src/BenchmarkDotNet/Code/DeclarationsProvider.cs index 21e3ce54c1..7528e8ed62 100644 --- a/src/BenchmarkDotNet/Code/DeclarationsProvider.cs +++ b/src/BenchmarkDotNet/Code/DeclarationsProvider.cs @@ -63,7 +63,7 @@ private string GetMethodName(MethodInfo method) (method.ReturnType.GetGenericTypeDefinition() == typeof(Task<>) || method.ReturnType.GetGenericTypeDefinition() == typeof(ValueTask<>)))) { - return $"() => awaitHelper.GetResult({method.Name}())"; + return $"() => BenchmarkDotNet.Helpers.AwaitHelper.GetResult({method.Name}())"; } return method.Name; @@ -150,9 +150,9 @@ internal class TaskDeclarationsProvider : VoidDeclarationsProvider public TaskDeclarationsProvider(Descriptor descriptor) : base(descriptor) { } public override string WorkloadMethodDelegate(string passArguments) - => $"({passArguments}) => {{ awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; + => $"({passArguments}) => {{ BenchmarkDotNet.Helpers.AwaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; - public override string GetWorkloadMethodCall(string passArguments) => $"awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; + public override string GetWorkloadMethodCall(string passArguments) => $"BenchmarkDotNet.Helpers.AwaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; protected override Type WorkloadMethodReturnType => typeof(void); } @@ -167,8 +167,8 @@ public GenericTaskDeclarationsProvider(Descriptor descriptor) : base(descriptor) protected override Type WorkloadMethodReturnType => Descriptor.WorkloadMethod.ReturnType.GetTypeInfo().GetGenericArguments().Single(); public override string WorkloadMethodDelegate(string passArguments) - => $"({passArguments}) => {{ return awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; + => $"({passArguments}) => {{ return BenchmarkDotNet.Helpers.AwaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments})); }}"; - public override string GetWorkloadMethodCall(string passArguments) => $"awaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; + public override string GetWorkloadMethodCall(string passArguments) => $"BenchmarkDotNet.Helpers.AwaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))"; } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs index 85b9829b18..0863f06c57 100644 --- a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -6,14 +6,19 @@ namespace BenchmarkDotNet.Helpers { - public class AwaitHelper + public static class AwaitHelper { private class ValueTaskWaiter { + // We use thread static field so that multiple threads can use individual lock object and callback. + [ThreadStatic] + private static ValueTaskWaiter ts_current; + internal static ValueTaskWaiter Current => ts_current ??= new ValueTaskWaiter(); + private readonly Action awaiterCallback; private bool awaiterCompleted; - internal ValueTaskWaiter() + private ValueTaskWaiter() { awaiterCallback = AwaiterCallback; } @@ -28,80 +33,70 @@ private void AwaiterCallback() } // Hook up a callback instead of converting to Task to prevent extra allocations on each benchmark run. - internal void GetResult(ValueTask task) + internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) { - // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. - var awaiter = task.ConfigureAwait(false).GetAwaiter(); - if (!awaiter.IsCompleted) + lock (this) { - lock (this) + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) - { - System.Threading.Monitor.Wait(this); - } + System.Threading.Monitor.Wait(this); } } - awaiter.GetResult(); } - internal T GetResult(ValueTask task) + internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) { - // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. - var awaiter = task.ConfigureAwait(false).GetAwaiter(); - if (!awaiter.IsCompleted) + lock (this) { - lock (this) + awaiterCompleted = false; + awaiter.UnsafeOnCompleted(awaiterCallback); + // Check if the callback executed synchronously before blocking. + if (!awaiterCompleted) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) - { - System.Threading.Monitor.Wait(this); - } + System.Threading.Monitor.Wait(this); } } - return awaiter.GetResult(); - } - } - - // We use thread static field so that multiple threads can use individual lock object and callback. - [ThreadStatic] - private static ValueTaskWaiter ts_valueTaskWaiter; - - private ValueTaskWaiter CurrentValueTaskWaiter - { - get - { - if (ts_valueTaskWaiter == null) - { - ts_valueTaskWaiter = new ValueTaskWaiter(); - } - return ts_valueTaskWaiter; } } // we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way, // and will eventually throw actual exception, not aggregated one - public void GetResult(Task task) => task.GetAwaiter().GetResult(); + public static void GetResult(Task task) => task.GetAwaiter().GetResult(); - public T GetResult(Task task) => task.GetAwaiter().GetResult(); + public static T GetResult(Task task) => task.GetAwaiter().GetResult(); // ValueTask can be backed by an IValueTaskSource that only supports asynchronous awaits, so we have to hook up a callback instead of calling .GetAwaiter().GetResult() like we do for Task. // The alternative is to convert it to Task using .AsTask(), but that causes allocations which we must avoid for memory diagnoser. - public void GetResult(ValueTask task) => CurrentValueTaskWaiter.GetResult(task); + public static void GetResult(ValueTask task) + { + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) + { + ValueTaskWaiter.Current.Wait(awaiter); + } + awaiter.GetResult(); + } - public T GetResult(ValueTask task) => CurrentValueTaskWaiter.GetResult(task); + public static T GetResult(ValueTask task) + { + // Don't continue on the captured context, as that may result in a deadlock if the user runs this in-process. + var awaiter = task.ConfigureAwait(false).GetAwaiter(); + if (!awaiter.IsCompleted) + { + ValueTaskWaiter.Current.Wait(awaiter); + } + return awaiter.GetResult(); + } internal static MethodInfo GetGetResultMethod(Type taskType) { if (!taskType.IsGenericType) { - return typeof(AwaitHelper).GetMethod(nameof(AwaitHelper.GetResult), BindingFlags.Public | BindingFlags.Instance, null, new Type[1] { taskType }, null); + return typeof(AwaitHelper).GetMethod(nameof(AwaitHelper.GetResult), BindingFlags.Public | BindingFlags.Static, null, new Type[1] { taskType }, null); } Type compareType = taskType.GetGenericTypeDefinition() == typeof(ValueTask<>) ? typeof(ValueTask<>) @@ -116,7 +111,7 @@ internal static MethodInfo GetGetResultMethod(Type taskType) .ReturnType .GetMethod(nameof(TaskAwaiter.GetResult), BindingFlags.Public | BindingFlags.Instance) .ReturnType; - return typeof(AwaitHelper).GetMethods(BindingFlags.Public | BindingFlags.Instance) + return typeof(AwaitHelper).GetMethods(BindingFlags.Public | BindingFlags.Static) .First(m => { if (m.Name != nameof(AwaitHelper.GetResult)) return false; diff --git a/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs b/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs index 65b016a0dd..6d601e6495 100644 --- a/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs +++ b/src/BenchmarkDotNet/Helpers/Reflection.Emit/IlGeneratorStatementExtensions.cs @@ -42,37 +42,6 @@ public static void EmitVoidReturn(this ILGenerator ilBuilder, MethodBuilder meth ilBuilder.Emit(OpCodes.Ret); } - public static void EmitSetFieldToNewInstance( - this ILGenerator ilBuilder, - FieldBuilder field, - Type instanceType) - { - if (field.IsStatic) - throw new ArgumentException("The field should be instance field", nameof(field)); - - if (instanceType != null) - { - /* - IL_0006: ldarg.0 - IL_0007: newobj instance void BenchmarkDotNet.Helpers.AwaitHelper::.ctor() - IL_000c: stfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Autogenerated.Runnable_0::awaitHelper - */ - var ctor = instanceType.GetConstructor(Array.Empty()); - if (ctor == null) - throw new InvalidOperationException($"Bug: instanceType {instanceType.Name} does not have a 0-parameter accessible constructor."); - - ilBuilder.Emit(OpCodes.Ldarg_0); - ilBuilder.Emit(OpCodes.Newobj, ctor); - ilBuilder.Emit(OpCodes.Stfld, field); - } - else - { - ilBuilder.Emit(OpCodes.Ldarg_0); - ilBuilder.Emit(OpCodes.Ldnull); - ilBuilder.Emit(OpCodes.Stfld, field); - } - } - public static void EmitSetDelegateToThisField( this ILGenerator ilBuilder, FieldBuilder delegateField, diff --git a/src/BenchmarkDotNet/Templates/BenchmarkType.txt b/src/BenchmarkDotNet/Templates/BenchmarkType.txt index 4bff1cdd98..f17737d646 100644 --- a/src/BenchmarkDotNet/Templates/BenchmarkType.txt +++ b/src/BenchmarkDotNet/Templates/BenchmarkType.txt @@ -57,8 +57,6 @@ public Runnable_$ID$() { - awaitHelper = new BenchmarkDotNet.Helpers.AwaitHelper(); - globalSetupAction = $GlobalSetupMethodName$; globalCleanupAction = $GlobalCleanupMethodName$; iterationSetupAction = $IterationSetupMethodName$; @@ -68,8 +66,6 @@ $InitializeArgumentFields$ } - private readonly BenchmarkDotNet.Helpers.AwaitHelper awaitHelper; - private System.Action globalSetupAction; private System.Action globalCleanupAction; private System.Action iterationSetupAction; diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs index 12e4072d0b..e0618cb642 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs @@ -248,7 +248,6 @@ private static void EmitNoArgsMethodCallPopReturn( private ConsumableTypeInfo consumableInfo; private ConsumeEmitter consumeEmitter; - private FieldBuilder awaitHelperField; private FieldBuilder globalSetupActionField; private FieldBuilder globalCleanupActionField; private FieldBuilder iterationSetupActionField; @@ -414,8 +413,6 @@ private Type EmitWorkloadDelegateType() private void DefineFields() { - awaitHelperField = - runnableBuilder.DefineField(AwaitHelperFieldName, typeof(Helpers.AwaitHelper), FieldAttributes.Private | FieldAttributes.InitOnly); globalSetupActionField = runnableBuilder.DefineField(GlobalSetupActionFieldName, typeof(Action), FieldAttributes.Private); globalCleanupActionField = @@ -588,13 +585,6 @@ private MethodInfo EmitWorkloadImplementation(string methodName) var ilBuilder = methodBuilder.GetILGenerator(); - /* - IL_0007: ldarg.0 - IL_0008: ldfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Helpers.Runnable_0::awaitHelper - */ - ilBuilder.Emit(OpCodes.Ldarg_0); - ilBuilder.Emit(OpCodes.Ldfld, awaitHelperField); - /* IL_0026: ldarg.0 IL_0027: ldloc.0 @@ -609,11 +599,11 @@ private MethodInfo EmitWorkloadImplementation(string methodName) ilBuilder.Emit(OpCodes.Call, Descriptor.WorkloadMethod); /* - // awaitHelper.GetResult(...); - IL_000e: callvirt instance void BenchmarkDotNet.Helpers.AwaitHelper::GetResult(class [System.Private.CoreLib]System.Threading.Tasks.Task) + // BenchmarkDotNet.Helpers.AwaitHelper.GetResult(...); + IL_000e: call !!0 BenchmarkDotNet.Helpers.AwaitHelper::GetResult(valuetype [System.Runtime]System.Threading.Tasks.ValueTask`1) */ - ilBuilder.Emit(OpCodes.Callvirt, consumableInfo.GetResultMethod); + ilBuilder.Emit(OpCodes.Call, consumableInfo.GetResultMethod); /* IL_0014: ret @@ -852,16 +842,6 @@ .locals init ( */ EmitLoadArgFieldsToLocals(ilBuilder, argLocals, skipFirstArg); - if (consumableInfo.IsAwaitable) - { - /* - IL_0026: ldarg.0 - IL_0027: ldfld class BenchmarkDotNet.Helpers.AwaitHelper BenchmarkDotNet.Helpers.Runnable_0::awaitHelper - */ - ilBuilder.Emit(OpCodes.Ldarg_0); - ilBuilder.Emit(OpCodes.Ldfld, awaitHelperField); - } - /* IL_0026: ldarg.0 IL_0027: ldloc.0 @@ -880,10 +860,10 @@ .locals init ( if (consumableInfo.IsAwaitable) { /* - // awaitHelper.GetResult(...); - IL_0036: callvirt instance void BenchmarkDotNet.Helpers.AwaitHelper::GetResult(class [System.Private.CoreLib]System.Threading.Tasks.Task) + // BenchmarkDotNet.Helpers.AwaitHelper.GetResult(...); + IL_000e: call !!0 BenchmarkDotNet.Helpers.AwaitHelper::GetResult(valuetype [System.Runtime]System.Threading.Tasks.ValueTask`1) */ - ilBuilder.Emit(OpCodes.Callvirt, consumableInfo.GetResultMethod); + ilBuilder.Emit(OpCodes.Call, consumableInfo.GetResultMethod); } /* @@ -946,7 +926,6 @@ private void EmitCtorBody() consumeEmitter.OnEmitCtorBody(ctorMethod, ilBuilder); - ilBuilder.EmitSetFieldToNewInstance(awaitHelperField, typeof(Helpers.AwaitHelper)); ilBuilder.EmitSetDelegateToThisField(globalSetupActionField, globalSetupMethod); ilBuilder.EmitSetDelegateToThisField(globalCleanupActionField, globalCleanupMethod); ilBuilder.EmitSetDelegateToThisField(iterationSetupActionField, iterationSetupMethod); diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs index 920fabb03e..c6e8cd8ae1 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Runnable/RunnableConstants.cs @@ -16,7 +16,6 @@ public class RunnableConstants public const string ArgFieldPrefix = "__argField"; public const string ArgParamPrefix = "arg"; - public const string AwaitHelperFieldName = "awaitHelper"; public const string GlobalSetupActionFieldName = "globalSetupAction"; public const string GlobalCleanupActionFieldName = "globalCleanupAction"; public const string IterationSetupActionFieldName = "iterationSetupAction"; diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs index fda7d0eef5..aee0a8f998 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs @@ -89,7 +89,6 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionTask : BenchmarkActionBase { - private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func startTaskCallback; private readonly Action callback; private readonly Action unrolledCallback; @@ -119,7 +118,7 @@ public BenchmarkActionTask(object instance, MethodInfo method, int unrollFactor) private void Overhead() { } // must be kept in sync with TaskDeclarationsProvider.TargetMethodDelegate - private void ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); + private void ExecuteBlocking() => Helpers.AwaitHelper.GetResult(startTaskCallback.Invoke()); [MethodImpl(CodeGenHelper.AggressiveOptimizationOption)] private void WorkloadActionUnroll(long repeatCount) @@ -138,7 +137,6 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionTask : BenchmarkActionBase { - private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func> startTaskCallback; private readonly Func callback; private readonly Func unrolledCallback; @@ -167,7 +165,7 @@ public BenchmarkActionTask(object instance, MethodInfo method, int unrollFactor) private T Overhead() => default; // must be kept in sync with GenericTaskDeclarationsProvider.TargetMethodDelegate - private T ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); + private T ExecuteBlocking() => Helpers.AwaitHelper.GetResult(startTaskCallback.Invoke()); private void InvokeSingleHardcoded() => result = callback(); @@ -190,7 +188,6 @@ private void WorkloadActionNoUnroll(long repeatCount) internal class BenchmarkActionValueTask : BenchmarkActionBase { - private readonly Helpers.AwaitHelper awaitHelper = new Helpers.AwaitHelper(); private readonly Func> startTaskCallback; private readonly Func callback; private readonly Func unrolledCallback; @@ -220,7 +217,7 @@ public BenchmarkActionValueTask(object instance, MethodInfo method, int unrollFa private T Overhead() => default; // must be kept in sync with GenericTaskDeclarationsProvider.TargetMethodDelegate - private T ExecuteBlocking() => awaitHelper.GetResult(startTaskCallback.Invoke()); + private T ExecuteBlocking() => Helpers.AwaitHelper.GetResult(startTaskCallback.Invoke()); private void InvokeSingleHardcoded() => result = callback(); diff --git a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs index 9eb40bad88..bb94aec254 100644 --- a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs +++ b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs @@ -12,8 +12,6 @@ namespace BenchmarkDotNet.Validators { public abstract class ExecutionValidatorBase : IValidator { - protected AwaitHelper awaitHelper = new (); - protected ExecutionValidatorBase(bool failOnError) { TreatsWarningsAsErrors = failOnError; @@ -134,7 +132,7 @@ private void TryToGetTaskResult(object result) } AwaitHelper.GetGetResultMethod(result.GetType()) - ?.Invoke(awaitHelper, new[] { result }); + ?.Invoke(null, new[] { result }); } private bool TryToSetParamsFields(object benchmarkTypeInstance, List errors) From 2806c798c62f705d7688ffedc95abdb5450567a9 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 Feb 2023 16:29:47 -0500 Subject: [PATCH 4/6] Add test case to make sure ValueTasks work properly with a race condition between `IsCompleted` and `OnCompleted`. Changed AwaitHelper to use `ManualResetEventSlim` instead of `Monitor.Wait`. --- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 50 +++++++------ .../AsyncBenchmarksTests.cs | 73 +++++++++++++++++++ 2 files changed, 99 insertions(+), 24 deletions(-) diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs index 0863f06c57..8eff9a806a 100644 --- a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; +using System.Threading; using System.Threading.Tasks; namespace BenchmarkDotNet.Helpers @@ -10,54 +11,55 @@ public static class AwaitHelper { private class ValueTaskWaiter { - // We use thread static field so that multiple threads can use individual lock object and callback. + // We use thread static field so that each thread uses its own individual callback and reset event. [ThreadStatic] private static ValueTaskWaiter ts_current; internal static ValueTaskWaiter Current => ts_current ??= new ValueTaskWaiter(); + // We cache the callback to prevent allocations for memory diagnoser. private readonly Action awaiterCallback; - private bool awaiterCompleted; + private readonly ManualResetEventSlim resetEvent; private ValueTaskWaiter() { - awaiterCallback = AwaiterCallback; - } - - private void AwaiterCallback() - { - lock (this) - { - awaiterCompleted = true; - System.Threading.Monitor.Pulse(this); - } + resetEvent = new (); + awaiterCallback = resetEvent.Set; } // Hook up a callback instead of converting to Task to prevent extra allocations on each benchmark run. internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) { - lock (this) + resetEvent.Reset(); + awaiter.UnsafeOnCompleted(awaiterCallback); + + // The fastest way to wait for completion is to spin a bit before waiting on the event. This is the same logic that Task.GetAwaiter().GetResult() uses. + var spinner = new SpinWait(); + while (!resetEvent.IsSet) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) + if (spinner.NextSpinWillYield) { - System.Threading.Monitor.Wait(this); + resetEvent.Wait(); + return; } + spinner.SpinOnce(); } } internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) { - lock (this) + resetEvent.Reset(); + awaiter.UnsafeOnCompleted(awaiterCallback); + + // The fastest way to wait for completion is to spin a bit before waiting on the event. This is the same logic that Task.GetAwaiter().GetResult() uses. + var spinner = new SpinWait(); + while (!resetEvent.IsSet) { - awaiterCompleted = false; - awaiter.UnsafeOnCompleted(awaiterCallback); - // Check if the callback executed synchronously before blocking. - if (!awaiterCompleted) + if (spinner.NextSpinWillYield) { - System.Threading.Monitor.Wait(this); + resetEvent.Wait(); + return; } + spinner.SpinOnce(); } } } diff --git a/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs b/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs index e9d1f3785e..d795b1a102 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AsyncBenchmarksTests.cs @@ -22,6 +22,23 @@ internal class ValueTaskSource : IValueTaskSource, IValueTaskSource public void SetResult(T result) => _core.SetResult(result); } + // This is used to test the case of ValueTaskAwaiter.IsCompleted returns false, then OnCompleted invokes the callback immediately because it happened to complete between the 2 calls. + internal class ValueTaskSourceCallbackOnly : IValueTaskSource, IValueTaskSource + { + private ManualResetValueTaskSourceCore _core; + + T IValueTaskSource.GetResult(short token) => _core.GetResult(token); + void IValueTaskSource.GetResult(short token) => _core.GetResult(token); + // Always return pending state so OnCompleted will be called. + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => ValueTaskSourceStatus.Pending; + ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => ValueTaskSourceStatus.Pending; + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + void IValueTaskSource.OnCompleted(Action continuation, object state, short token, ValueTaskSourceOnCompletedFlags flags) => _core.OnCompleted(continuation, state, token, flags); + public void Reset() => _core.Reset(); + public short Token => _core.Version; + public void SetResult(T result) => _core.SetResult(result); + } + public class AsyncBenchmarksTests : BenchmarkTestExecutor { public AsyncBenchmarksTests(ITestOutputHelper output) : base(output) { } @@ -41,6 +58,9 @@ public void TaskReturningMethodsAreAwaited() } } + [Fact] + public void TaskReturningMethodsAreAwaited_AlreadyComplete() => CanExecute(); + public class TaskDelayMethods { private readonly ValueTaskSource valueTaskSource = new (); @@ -89,5 +109,58 @@ public ValueTask ReturningGenericValueTaskBackByIValueTaskSource() return new ValueTask(valueTaskSource, valueTaskSource.Token); } } + + public class TaskImmediateMethods + { + private readonly ValueTaskSource valueTaskSource = new (); + private readonly ValueTaskSourceCallbackOnly valueTaskSourceCallbackOnly = new (); + + [Benchmark] + public Task ReturningTask() => Task.CompletedTask; + + [Benchmark] + public ValueTask ReturningValueTask() => new ValueTask(); + + [Benchmark] + public ValueTask ReturningValueTaskBackByIValueTaskSource() + { + valueTaskSource.Reset(); + valueTaskSource.SetResult(default); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } + + [Benchmark] + public ValueTask ReturningValueTaskBackByIValueTaskSource_ImmediateCallback() + { + valueTaskSourceCallbackOnly.Reset(); + valueTaskSourceCallbackOnly.SetResult(default); + return new ValueTask(valueTaskSourceCallbackOnly, valueTaskSourceCallbackOnly.Token); + } + + [Benchmark] + public async Task Awaiting() => await Task.CompletedTask; + + [Benchmark] + public Task ReturningGenericTask() => ReturningTask().ContinueWith(_ => default(int)); + + [Benchmark] + public ValueTask ReturningGenericValueTask() => new ValueTask(ReturningGenericTask()); + + [Benchmark] + public ValueTask ReturningGenericValueTaskBackByIValueTaskSource() + { + valueTaskSource.Reset(); + valueTaskSource.SetResult(default); + return new ValueTask(valueTaskSource, valueTaskSource.Token); + } + + [Benchmark] + public ValueTask ReturningGenericValueTaskBackByIValueTaskSource_ImmediateCallback() + { + valueTaskSourceCallbackOnly.Reset(); + valueTaskSourceCallbackOnly.SetResult(default); + return new ValueTask(valueTaskSourceCallbackOnly, valueTaskSourceCallbackOnly.Token); + } + } } } \ No newline at end of file From 2a5de176cd8136033d0befcf05c8a37381706432 Mon Sep 17 00:00:00 2001 From: Tim Date: Sat, 29 Jul 2023 23:35:26 -0400 Subject: [PATCH 5/6] Make `ValueTaskWaiter.Wait` generic. --- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 24 +++------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs index 8eff9a806a..82a59ae7b2 100644 --- a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -26,26 +26,7 @@ private ValueTaskWaiter() awaiterCallback = resetEvent.Set; } - // Hook up a callback instead of converting to Task to prevent extra allocations on each benchmark run. - internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) - { - resetEvent.Reset(); - awaiter.UnsafeOnCompleted(awaiterCallback); - - // The fastest way to wait for completion is to spin a bit before waiting on the event. This is the same logic that Task.GetAwaiter().GetResult() uses. - var spinner = new SpinWait(); - while (!resetEvent.IsSet) - { - if (spinner.NextSpinWillYield) - { - resetEvent.Wait(); - return; - } - spinner.SpinOnce(); - } - } - - internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter awaiter) + internal void Wait(TAwaiter awaiter) where TAwaiter : ICriticalNotifyCompletion { resetEvent.Reset(); awaiter.UnsafeOnCompleted(awaiterCallback); @@ -70,7 +51,8 @@ internal void Wait(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter public static T GetResult(Task task) => task.GetAwaiter().GetResult(); - // ValueTask can be backed by an IValueTaskSource that only supports asynchronous awaits, so we have to hook up a callback instead of calling .GetAwaiter().GetResult() like we do for Task. + // ValueTask can be backed by an IValueTaskSource that only supports asynchronous awaits, + // so we have to hook up a callback instead of calling .GetAwaiter().GetResult() like we do for Task. // The alternative is to convert it to Task using .AsTask(), but that causes allocations which we must avoid for memory diagnoser. public static void GetResult(ValueTask task) { From 584fe39511b2e3aecc4e5df81c023d77a6377efe Mon Sep 17 00:00:00 2001 From: Tim Date: Sun, 30 Jul 2023 00:34:14 -0400 Subject: [PATCH 6/6] Compare types directly. --- src/BenchmarkDotNet/Helpers/AwaitHelper.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs index 82a59ae7b2..8d16fb716a 100644 --- a/src/BenchmarkDotNet/Helpers/AwaitHelper.cs +++ b/src/BenchmarkDotNet/Helpers/AwaitHelper.cs @@ -100,8 +100,7 @@ internal static MethodInfo GetGetResultMethod(Type taskType) { if (m.Name != nameof(AwaitHelper.GetResult)) return false; Type paramType = m.GetParameters().First().ParameterType; - // We have to compare the types indirectly, == check doesn't work. - return paramType.Assembly == compareType.Assembly && paramType.Namespace == compareType.Namespace && paramType.Name == compareType.Name; + return paramType.IsGenericType && paramType.GetGenericTypeDefinition() == compareType; }) .MakeGenericMethod(new[] { resultType }); }