From fa1499a26e180e6070958230e75895251ecdd68e Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 19 Sep 2022 18:06:47 -0400 Subject: [PATCH 1/5] Added AwaitHelper to properly wait for ValueTasks. --- .../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 | 94 +++++------ .../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, 424 insertions(+), 185 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 d8f15f9138..7a215a4c93 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 0fde3ac1ee..d5fa11bce4 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/ConsumableTypeInfo.cs @@ -1,6 +1,8 @@ using BenchmarkDotNet.Engines; using JetBrains.Annotations; using System; +using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Threading.Tasks; @@ -17,28 +19,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) @@ -75,7 +73,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; } @@ -83,6 +80,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 7f9d47c62f..8d622fd187 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs @@ -246,6 +246,7 @@ private static void EmitNoArgsMethodCallPopReturn( private ConsumableTypeInfo consumableInfo; private ConsumeEmitter consumeEmitter; + private FieldBuilder awaitHelperField; private FieldBuilder globalSetupActionField; private FieldBuilder globalCleanupActionField; private FieldBuilder iterationSetupActionField; @@ -411,6 +412,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 = @@ -580,42 +583,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 @@ -830,19 +826,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(); @@ -866,30 +849,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); } /* @@ -952,6 +943,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 eef3ce8997..a139adbafc 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs @@ -69,6 +69,7 @@ private void InvokeMultipleHardcoded(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; @@ -97,7 +98,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()); private void InvokeMultipleHardcoded(long repeatCount) { @@ -108,6 +109,7 @@ private void InvokeMultipleHardcoded(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; @@ -135,7 +137,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(); @@ -150,6 +152,7 @@ private void InvokeMultipleHardcoded(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; @@ -178,7 +181,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 92d7422ae2..8c644c11f8 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 6975a01401..314e183d2b 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/AllSetupAndCleanupTest.cs @@ -49,13 +49,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) @@ -84,21 +91,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; @@ -116,24 +109,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; @@ -145,30 +124,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; @@ -180,7 +176,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); @@ -188,7 +184,7 @@ public async Task GlobalSetup() } [GlobalCleanup] - public async Task GlobalCleanup() + public async ValueTask GlobalCleanup() { await Console.Out.WriteLineAsync(GlobalCleanupCalled); @@ -199,22 +195,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; @@ -225,31 +208,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); } - [FactNotGitHubActionsWindows] - 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; @@ -260,19 +240,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 1d5619a27c..dbf44c6cc2 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 6ca7394f4d..49313c14db 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; @@ -597,5 +598,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 c58bf1f8db32a67480743914c3ba8e25f2611983 Mon Sep 17 00:00:00 2001 From: Tim Date: Sun, 25 Sep 2022 15:05:01 -0400 Subject: [PATCH 2/5] 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 b3d8e81a17bd8b59bc44dae035dbb2bef3518bef Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 26 Sep 2022 06:12:00 -0400 Subject: [PATCH 3/5] 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 7a215a4c93..d8f15f9138 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 8d622fd187..eade0c8981 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs @@ -246,7 +246,6 @@ private static void EmitNoArgsMethodCallPopReturn( private ConsumableTypeInfo consumableInfo; private ConsumeEmitter consumeEmitter; - private FieldBuilder awaitHelperField; private FieldBuilder globalSetupActionField; private FieldBuilder globalCleanupActionField; private FieldBuilder iterationSetupActionField; @@ -412,8 +411,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 = @@ -586,13 +583,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 @@ -607,11 +597,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 @@ -849,16 +839,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 @@ -877,10 +857,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); } /* @@ -943,7 +923,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 a139adbafc..39ec7f5b42 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs @@ -69,7 +69,6 @@ private void InvokeMultipleHardcoded(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; @@ -98,7 +97,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()); private void InvokeMultipleHardcoded(long repeatCount) { @@ -109,7 +108,6 @@ private void InvokeMultipleHardcoded(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; @@ -137,7 +135,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(); @@ -152,7 +150,6 @@ private void InvokeMultipleHardcoded(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; @@ -181,7 +178,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 8c644c11f8..b495e4d87f 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 43138da51194beb972009209ddd5985f860c941a Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 17 Feb 2023 16:29:47 -0500 Subject: [PATCH 4/5] 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 c51c4865d160f4b0e938ee1e7af33552b51b2af3 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 19 Sep 2022 18:06:47 -0400 Subject: [PATCH 5/5] Added missing ValueTask (non-generic) to InProcess (no emit) toolchains. --- .../NoEmit/BenchmarkActionFactory.cs | 3 + .../BenchmarkActionFactory_Implementations.cs | 39 ++++ .../InProcessTest.cs | 184 ++++++++++++++++-- 3 files changed, 205 insertions(+), 21 deletions(-) diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory.cs b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory.cs index 6b0f2468d8..565c8d8f15 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory.cs @@ -32,6 +32,9 @@ private static BenchmarkAction CreateCore( if (resultType == typeof(Task)) return new BenchmarkActionTask(resultInstance, targetMethod, unrollFactor); + if (resultType == typeof(ValueTask)) + return new BenchmarkActionValueTask(resultInstance, targetMethod, unrollFactor); + if (resultType.GetTypeInfo().IsGenericType) { var genericType = resultType.GetGenericTypeDefinition(); diff --git a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs index 39ec7f5b42..2ed1f7ccc6 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/BenchmarkActionFactory_Implementations.cs @@ -148,6 +148,45 @@ private void InvokeMultipleHardcoded(long repeatCount) public override object LastRunResult => result; } + internal class BenchmarkActionValueTask : BenchmarkActionBase + { + private readonly Func startTaskCallback; + private readonly Action callback; + private readonly Action unrolledCallback; + + public BenchmarkActionValueTask(object instance, MethodInfo method, int unrollFactor) + { + bool isIdle = method == null; + if (!isIdle) + { + startTaskCallback = CreateWorkload>(instance, method); + callback = ExecuteBlocking; + } + else + { + callback = Overhead; + } + + InvokeSingle = callback; + + unrolledCallback = Unroll(callback, unrollFactor); + InvokeMultiple = InvokeMultipleHardcoded; + + } + + // must be kept in sync with VoidDeclarationsProvider.IdleImplementation + private void Overhead() { } + + // must be kept in sync with TaskDeclarationsProvider.TargetMethodDelegate + private void ExecuteBlocking() => Helpers.AwaitHelper.GetResult(startTaskCallback.Invoke()); + + private void InvokeMultipleHardcoded(long repeatCount) + { + for (long i = 0; i < repeatCount; i++) + unrolledCallback(); + } + } + internal class BenchmarkActionValueTask : BenchmarkActionBase { private readonly Func> startTaskCallback; diff --git a/tests/BenchmarkDotNet.IntegrationTests/InProcessTest.cs b/tests/BenchmarkDotNet.IntegrationTests/InProcessTest.cs index a06870ad5e..5f720fff5f 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/InProcessTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/InProcessTest.cs @@ -45,6 +45,9 @@ public InProcessTest(ITestOutputHelper output) : base(output) [Fact] public void BenchmarkActionTaskSupported() => TestInvoke(x => x.InvokeOnceTaskAsync(), UnrollFactor, null); + [Fact] + public void BenchmarkActionValueTaskSupported() => TestInvoke(x => x.InvokeOnceValueTaskAsync(), UnrollFactor, null); + [Fact] public void BenchmarkActionRefTypeSupported() => TestInvoke(x => x.InvokeOnceRefType(), UnrollFactor, StringResult); @@ -57,6 +60,18 @@ public InProcessTest(ITestOutputHelper output) : base(output) [Fact] public void BenchmarkActionValueTaskOfTSupported() => TestInvoke(x => x.InvokeOnceValueTaskOfT(), UnrollFactor, DecimalResult); + [Fact] + public void BenchmarkActionGlobalSetupTaskSupported() => TestInvokeSetupCleanupTask(x => BenchmarkSetupCleanupTask.GlobalSetup(), UnrollFactor); + + [Fact] + public void BenchmarkActionGlobalCleanupTaskSupported() => TestInvokeSetupCleanupTask(x => x.GlobalCleanup(), UnrollFactor); + + [Fact] + public void BenchmarkActionGlobalSetupValueTaskSupported() => TestInvokeSetupCleanupValueTask(x => BenchmarkSetupCleanupValueTask.GlobalSetup(), UnrollFactor); + + [Fact] + public void BenchmarkActionGlobalCleanupValueTaskSupported() => TestInvokeSetupCleanupValueTask(x => x.GlobalCleanup(), UnrollFactor); + [Fact] public void BenchmarkDifferentPlatformReturnsValidationError() { @@ -83,30 +98,30 @@ private void TestInvoke(Expression> methodCall, int un // Run mode var action = BenchmarkActionFactory.CreateWorkload(descriptor, new BenchmarkAllCases(), unrollFactor); - TestInvoke(action, unrollFactor, false, null); + TestInvoke(action, unrollFactor, false, null, ref BenchmarkAllCases.Counter); // Idle mode action = BenchmarkActionFactory.CreateOverhead(descriptor, new BenchmarkAllCases(), unrollFactor); - TestInvoke(action, unrollFactor, true, null); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkAllCases.Counter); // GlobalSetup/GlobalCleanup action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkAllCases()); - TestInvoke(action, 1, false, null); + TestInvoke(action, 1, false, null, ref BenchmarkAllCases.Counter); action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkAllCases()); - TestInvoke(action, 1, false, null); + TestInvoke(action, 1, false, null, ref BenchmarkAllCases.Counter); // GlobalSetup/GlobalCleanup (empty) descriptor = new Descriptor(typeof(BenchmarkAllCases), targetMethod); action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkAllCases()); - TestInvoke(action, unrollFactor, true, null); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkAllCases.Counter); action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkAllCases()); - TestInvoke(action, unrollFactor, true, null); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkAllCases.Counter); // Dummy (just in case something may broke) action = BenchmarkActionFactory.CreateDummy(); - TestInvoke(action, unrollFactor, true, null); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkAllCases.Counter); action = BenchmarkActionFactory.CreateDummy(); - TestInvoke(action, unrollFactor, true, null); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkAllCases.Counter); } [AssertionMethod] @@ -117,7 +132,7 @@ private void TestInvoke(Expression> methodCall, in // Run mode var action = BenchmarkActionFactory.CreateWorkload(descriptor, new BenchmarkAllCases(), unrollFactor); - TestInvoke(action, unrollFactor, false, expectedResult); + TestInvoke(action, unrollFactor, false, expectedResult, ref BenchmarkAllCases.Counter); // Idle mode @@ -126,15 +141,83 @@ private void TestInvoke(Expression> methodCall, in object idleExpected; if (isValueTask) idleExpected = GetDefault(typeof(T).GetGenericArguments()[0]); + else if (expectedResult == null || typeof(T) == typeof(Task) || typeof(T) == typeof(ValueTask)) + idleExpected = null; else if (typeof(T).GetTypeInfo().IsValueType) idleExpected = 0; - else if (expectedResult == null || typeof(T) == typeof(Task)) - idleExpected = null; else idleExpected = GetDefault(expectedResult.GetType()); action = BenchmarkActionFactory.CreateOverhead(descriptor, new BenchmarkAllCases(), unrollFactor); - TestInvoke(action, unrollFactor, true, idleExpected); + TestInvoke(action, unrollFactor, true, idleExpected, ref BenchmarkAllCases.Counter); + } + + [AssertionMethod] + private void TestInvokeSetupCleanupTask(Expression> methodCall, int unrollFactor) + { + var targetMethod = ((MethodCallExpression) methodCall.Body).Method; + var descriptor = new Descriptor(typeof(BenchmarkSetupCleanupTask), targetMethod, targetMethod, targetMethod, targetMethod, targetMethod); + + // Run mode + var action = BenchmarkActionFactory.CreateWorkload(descriptor, new BenchmarkSetupCleanupTask(), unrollFactor); + TestInvoke(action, unrollFactor, false, null, ref BenchmarkSetupCleanupTask.Counter); + + // Idle mode + action = BenchmarkActionFactory.CreateOverhead(descriptor, new BenchmarkSetupCleanupTask(), unrollFactor); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupTask.Counter); + + // GlobalSetup/GlobalCleanup + action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkSetupCleanupTask()); + TestInvoke(action, 1, false, null, ref BenchmarkSetupCleanupTask.Counter); + action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkSetupCleanupTask()); + TestInvoke(action, 1, false, null, ref BenchmarkSetupCleanupTask.Counter); + + // GlobalSetup/GlobalCleanup (empty) + descriptor = new Descriptor(typeof(BenchmarkSetupCleanupTask), targetMethod); + action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkSetupCleanupTask()); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupTask.Counter); + action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkSetupCleanupTask()); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupTask.Counter); + + // Dummy (just in case something may broke) + action = BenchmarkActionFactory.CreateDummy(); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupTask.Counter); + action = BenchmarkActionFactory.CreateDummy(); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupTask.Counter); + } + + [AssertionMethod] + private void TestInvokeSetupCleanupValueTask(Expression> methodCall, int unrollFactor) + { + var targetMethod = ((MethodCallExpression) methodCall.Body).Method; + var descriptor = new Descriptor(typeof(BenchmarkSetupCleanupValueTask), targetMethod, targetMethod, targetMethod, targetMethod, targetMethod); + + // Run mode + var action = BenchmarkActionFactory.CreateWorkload(descriptor, new BenchmarkSetupCleanupValueTask(), unrollFactor); + TestInvoke(action, unrollFactor, false, null, ref BenchmarkSetupCleanupValueTask.Counter); + + // Idle mode + action = BenchmarkActionFactory.CreateOverhead(descriptor, new BenchmarkSetupCleanupValueTask(), unrollFactor); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupValueTask.Counter); + + // GlobalSetup/GlobalCleanup + action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkSetupCleanupValueTask()); + TestInvoke(action, 1, false, null, ref BenchmarkSetupCleanupValueTask.Counter); + action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkSetupCleanupValueTask()); + TestInvoke(action, 1, false, null, ref BenchmarkSetupCleanupValueTask.Counter); + + // GlobalSetup/GlobalCleanup (empty) + descriptor = new Descriptor(typeof(BenchmarkSetupCleanupValueTask), targetMethod); + action = BenchmarkActionFactory.CreateGlobalSetup(descriptor, new BenchmarkSetupCleanupValueTask()); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupValueTask.Counter); + action = BenchmarkActionFactory.CreateGlobalCleanup(descriptor, new BenchmarkSetupCleanupValueTask()); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupValueTask.Counter); + + // Dummy (just in case something may broke) + action = BenchmarkActionFactory.CreateDummy(); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupValueTask.Counter); + action = BenchmarkActionFactory.CreateDummy(); + TestInvoke(action, unrollFactor, true, null, ref BenchmarkSetupCleanupValueTask.Counter); } private static object GetDefault(Type type) @@ -147,36 +230,36 @@ private static object GetDefault(Type type) } [AssertionMethod] - private void TestInvoke(BenchmarkAction benchmarkAction, int unrollFactor, bool isIdle, object expectedResult) + private void TestInvoke(BenchmarkAction benchmarkAction, int unrollFactor, bool isIdle, object expectedResult, ref int counter) { try { - BenchmarkAllCases.Counter = 0; + counter = 0; if (isIdle) { benchmarkAction.InvokeSingle(); - Assert.Equal(0, BenchmarkAllCases.Counter); + Assert.Equal(0, counter); benchmarkAction.InvokeMultiple(0); - Assert.Equal(0, BenchmarkAllCases.Counter); + Assert.Equal(0, counter); benchmarkAction.InvokeMultiple(11); - Assert.Equal(0, BenchmarkAllCases.Counter); + Assert.Equal(0, counter); } else { benchmarkAction.InvokeSingle(); - Assert.Equal(1, BenchmarkAllCases.Counter); + Assert.Equal(1, counter); benchmarkAction.InvokeMultiple(0); - Assert.Equal(1, BenchmarkAllCases.Counter); + Assert.Equal(1, counter); benchmarkAction.InvokeMultiple(11); - Assert.Equal(BenchmarkAllCases.Counter, 1 + unrollFactor * 11); + Assert.Equal(1 + unrollFactor * 11, counter); } Assert.Equal(benchmarkAction.LastRunResult, expectedResult); } finally { - BenchmarkAllCases.Counter = 0; + counter = 0; } } @@ -244,6 +327,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() { @@ -273,5 +363,57 @@ public ValueTask InvokeOnceValueTaskOfT() return new ValueTask(DecimalResult); } } + + [UsedImplicitly(ImplicitUseTargetFlags.WithMembers)] + public class BenchmarkSetupCleanupTask + { + public static int Counter; + + [GlobalSetup] + public static async Task GlobalSetup() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + + [GlobalCleanup] + public async Task GlobalCleanup() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + + [Benchmark] + public void InvokeOnceVoid() + { + Interlocked.Increment(ref Counter); + } + } + + [UsedImplicitly(ImplicitUseTargetFlags.WithMembers)] + public class BenchmarkSetupCleanupValueTask + { + public static int Counter; + + [GlobalSetup] + public static async ValueTask GlobalSetup() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + + [GlobalCleanup] + public async ValueTask GlobalCleanup() + { + await Task.Yield(); + Interlocked.Increment(ref Counter); + } + + [Benchmark] + public void InvokeOnceVoid() + { + Interlocked.Increment(ref Counter); + } + } } } \ No newline at end of file