Refactor Engine.Run to call GlobalSetup/Cleanup

src/BenchmarkDotNet/Engines/Engine.cs

@@ -1,12 +1,9 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Threading;
 using BenchmarkDotNet.Characteristics;
-using BenchmarkDotNet.Environments;
-using BenchmarkDotNet.Helpers;
 using BenchmarkDotNet.Jobs;
 using BenchmarkDotNet.Portability;
 using BenchmarkDotNet.Reports;
@@ -62,26 +59,37 @@ internal Engine(EngineParameters engineParameters)
             random = new Random(12345); // we are using constant seed to try to get repeatable results
         }
 
-        public void Dispose()
+        public RunResults Run()
         {
+            Parameters.GlobalSetupAction.Invoke();
+            bool didThrow = false;
             try
             {
-                Parameters.GlobalCleanupAction.Invoke();
+                return RunCore();
             }
-            catch (Exception e)
+            catch
             {
-                Host.SendError("Exception during GlobalCleanup!");
-                Host.SendError(e.Message);
-
-                // we don't rethrow because this code is executed in a finally block
-                // and it could possibly overwrite current exception #1045
+                didThrow = true;
+                throw;
+            }
+            finally
+            {
+                try
+                {
+                    Parameters.GlobalCleanupAction.Invoke();
+                }
+                // We only catch if the benchmark threw to not overwrite the exception. #1045
+                catch (Exception e) when (didThrow)
+                {
+                    Host.SendError($"Exception during GlobalCleanup!{Environment.NewLine}{e}");
+                }
             }
         }
 
         // AggressiveOptimization forces the method to go straight to tier1 JIT, and will never be re-jitted,
         // eliminating tiered JIT as a potential variable in measurements.
         [MethodImpl(CodeGenHelper.AggressiveOptimizationOption)]
-        public RunResults Run()
+        private RunResults RunCore()
         {
             var measurements = new List<Measurement>();
 

src/BenchmarkDotNet/Engines/EngineFactory.cs

@@ -1,15 +1,7 @@
-namespace BenchmarkDotNet.Engines
-{
-    public class EngineFactory : IEngineFactory
-    {
-        public IEngine CreateReadyToRun(EngineParameters engineParameters)
-        {
-            var engine = new Engine(engineParameters);
-
-            // TODO: Move GlobalSetup/Cleanup to Engine.Run.
-            engine.Parameters.GlobalSetupAction.Invoke(); // whatever the settings are, we MUST call global setup here, the global cleanup is part of Engine's Dispose
+namespace BenchmarkDotNet.Engines;
 
-            return engine;
-        }
-    }
-}
+public class EngineFactory : IEngineFactory
+{
+    public IEngine Create(EngineParameters engineParameters)
+        => new Engine(engineParameters);
+}

src/BenchmarkDotNet/Engines/IEngine.cs

@@ -1,9 +1,6 @@
-using System;
+namespace BenchmarkDotNet.Engines;
 
-namespace BenchmarkDotNet.Engines
+public interface IEngine
 {
-    public interface IEngine : IDisposable
-    {
-        RunResults Run();
-    }
+    RunResults Run();
 }

src/BenchmarkDotNet/Engines/IEngineFactory.cs

@@ -2,6 +2,6 @@ namespace BenchmarkDotNet.Engines
 {
     public interface IEngineFactory
     {
-        IEngine CreateReadyToRun(EngineParameters engineParameters);
+        IEngine Create(EngineParameters engineParameters);
     }
 }

src/BenchmarkDotNet/Templates/BenchmarkType.txt

@@ -53,14 +53,10 @@
                 InProcessDiagnoserHandler = compositeInProcessDiagnoserHandler
             };
 
-            using (global::BenchmarkDotNet.Engines.IEngine engine = new $EngineFactoryType$().CreateReadyToRun(engineParameters))
-            {
-                global::BenchmarkDotNet.Engines.RunResults results = engine.Run();
-
-                host.ReportResults(results); // printing costs memory, do this after runs
+            global::BenchmarkDotNet.Engines.RunResults results = new $EngineFactoryType$().Create(engineParameters).Run();
+            host.ReportResults(results); // printing costs memory, do this after runs
 
-                instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
-            }
+            instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
             compositeInProcessDiagnoserHandler.Handle(global::BenchmarkDotNet.Engines.BenchmarkSignal.AfterEngine);
         }
 

src/BenchmarkDotNet/Toolchains/InProcess/Emit/Implementation/Emitters/RunnableEmitter.cs

@@ -873,7 +873,7 @@ class [BenchmarkDotNet]BenchmarkDotNet.Toolchains.Parameters.ExecuteParameters p
             var argsExceptInstance = prepareForRunMethodTemplate
                 .GetParameters()
                 .Skip(1)
-                .Select(p => (ParameterInfo)new EmitParameterInfo(p.Position - 1, p.Name, p.ParameterType, p.Attributes, null))
+                .Select(p => (ParameterInfo) new EmitParameterInfo(p.Position - 1, p.Name, p.ParameterType, p.Attributes, null))
                 .ToArray();
             var methodBuilder = runnableBuilder.DefineStaticMethod(
                 RunMethodName,
@@ -892,15 +892,13 @@ .locals init (
                     [1] class [BenchmarkDotNet]BenchmarkDotNet.Jobs.Job,
                     [2] class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters,
                     [3] class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory,
-                    [4] class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine,
-                    [5] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
+                    [4] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
                 )
              */
             var instanceLocal = ilBuilder.DeclareLocal(runnableBuilder);
             var jobLocal = ilBuilder.DeclareLocal(typeof(Job));
             var engineParametersLocal = ilBuilder.DeclareLocal(typeof(EngineParameters));
             var engineFactoryLocal = ilBuilder.DeclareLocal(typeof(IEngineFactory));
-            var engineLocal = ilBuilder.DeclareLocal(typeof(IEngine));
             var runResultsLocal = ilBuilder.DeclareLocal(typeof(RunResults));
 
             /*
@@ -961,82 +959,48 @@ [5] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
             ilBuilder.EmitVoidReturn(methodBuilder);
 
             /*
-                // using (IEngine engine = engineFactory.CreateReadyToRun(engineParameters))
+                // RunResults results = engineFactory.Create(engineParameters).Run();
                 IL_0026: ldloc.3
                 IL_0027: ldloc.2
-                IL_0028: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory::CreateReadyToRun(class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters)
-                IL_002d: stloc.s 4
+                IL_0028: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory::Create(class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters)
+                IL_002d: callvirt instance valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine::Run()
+                IL_0032: stloc.s 4
              */
-            var createReadyToRunMethod = typeof(IEngineFactory).GetMethod(nameof(IEngineFactory.CreateReadyToRun))
-                ?? throw new MissingMemberException(nameof(IEngineFactory.CreateReadyToRun));
+            var createReadyToRunMethod = typeof(IEngineFactory).GetMethod(nameof(IEngineFactory.Create))
+                ?? throw new MissingMemberException(nameof(IEngineFactory.Create));
+            var runMethodImpl = typeof(IEngine).GetMethod(nameof(IEngine.Run))
+                ?? throw new MissingMemberException(nameof(IEngine.Run));
             ilBuilder.MarkLabel(notNullLabel);
             ilBuilder.EmitLdloc(engineFactoryLocal);
             ilBuilder.EmitLdloc(engineParametersLocal);
             ilBuilder.Emit(OpCodes.Callvirt, createReadyToRunMethod);
-            ilBuilder.EmitStloc(engineLocal);
-
-            // .try
-            // {
-            ilBuilder.BeginExceptionBlock();
-            {
-                /*
-                    // RunResults results = engine.Run();
-                    IL_002f: ldloc.s 4
-                    IL_0031: callvirt instance valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine::Run()
-                    IL_0036: stloc.s 5
-                 */
-                var runMethodImpl = typeof(IEngine).GetMethod(nameof(IEngine.Run))
-                    ?? throw new MissingMemberException(nameof(IEngine.Run));
-                ilBuilder.EmitLdloc(engineLocal);
-                ilBuilder.Emit(OpCodes.Callvirt, runMethodImpl);
-                ilBuilder.EmitStloc(runResultsLocal);
-                /*
-                    // host.ReportResults(results);
-                    IL_0038: ldarg.1
-                    IL_0039: ldloc.s 5
-                    IL_003b: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Engines.IHost::ReportResults(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults)
-                 */
-                var reportResultsMethod = typeof(IHost).GetMethod(nameof(IHost.ReportResults))
-                    ?? throw new MissingMemberException(nameof(IHost.ReportResults));
-                ilBuilder.EmitLdarg(hostArg);
-                ilBuilder.EmitLdloc(runResultsLocal);
-                ilBuilder.Emit(OpCodes.Callvirt, reportResultsMethod);
-                /*
-                    // instance.__TrickTheJIT__();
-                    IL_0040: ldloc.0
-                    IL_0041: callvirt instance void BenchmarkDotNet.Autogenerated.ReplaceMe.Runnable0::__TrickTheJIT__()
-                 */
-                ilBuilder.Emit(OpCodes.Ldloc_0);
-                ilBuilder.Emit(OpCodes.Callvirt, trickTheJitMethod);
-            }
-            // finally
-            // {
-            ilBuilder.BeginFinallyBlock();
-            {
-                /*
-                    IL_0048: ldloc.s 4
-                    IL_004a: brfalse.s IL_0053
-                    IL_004c: ldloc.s 4
-                    IL_004e: callvirt instance void [mscorlib]System.IDisposable::Dispose()
-                 */
-                var disposeMethod = typeof(IDisposable).GetMethod(nameof(IDisposable.Dispose))
-                    ?? throw new MissingMemberException(nameof(IDisposable.Dispose));
-                var disposeNullLabel = ilBuilder.DefineLabel();
-                ilBuilder.EmitLdloc(engineLocal);
-                ilBuilder.Emit(OpCodes.Brfalse_S, disposeNullLabel);
-                ilBuilder.EmitLdloc(engineLocal);
-                ilBuilder.Emit(OpCodes.Callvirt, disposeMethod);
-
-                ilBuilder.MarkLabel(disposeNullLabel);
-                ilBuilder.EndExceptionBlock();
-            }
+            ilBuilder.Emit(OpCodes.Callvirt, runMethodImpl);
+            ilBuilder.EmitStloc(runResultsLocal);
+            /*
+                // host.ReportResults(runResults);
+                IL_0034: ldarg.0
+                IL_0035: ldloc.s 4
+                IL_0037: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Engines.IHost::ReportResults(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults)
+            */
 
+            var reportResultsMethod = typeof(IHost).GetMethod(nameof(IHost.ReportResults))
+                ?? throw new MissingMemberException(nameof(IHost.ReportResults));
+            ilBuilder.EmitLdarg(hostArg);
+            ilBuilder.EmitLdloc(runResultsLocal);
+            ilBuilder.Emit(OpCodes.Callvirt, reportResultsMethod);
+            /*
+                // runnable_.__TrickTheJIT__();
+                IL_003c: ldloc.0
+                IL_003d: callvirt instance void BenchmarkDotNet.Autogenerated.ReplaceMe.Runnable_0::__TrickTheJIT__()
+             */
+            ilBuilder.Emit(OpCodes.Ldloc_0);
+            ilBuilder.Emit(OpCodes.Callvirt, trickTheJitMethod);
             /*
                 // engineParameters.InProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
-                IL_0054: ldloc.2
-                IL_0055: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters::get_InProcessDiagnoserHandler()
-                IL_005a: ldc.i4.3
-                IL_005b: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler::Handle(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.BenchmarkSignal)
+                IL_0042: ldloc.2
+                IL_0043: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters::get_InProcessDiagnoserHandler()
+                IL_0048: ldc.i4.5
+                IL_0049: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler::Handle(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.BenchmarkSignal)
             */
             ilBuilder.EmitLdloc(engineParametersLocal);
             ilBuilder.Emit(OpCodes.Callvirt, typeof(EngineParameters).GetProperty(nameof(EngineParameters.InProcessDiagnoserHandler)).GetGetMethod());

src/BenchmarkDotNet/Toolchains/InProcess/NoEmit/InProcessNoEmitRunner.cs

@@ -166,14 +166,12 @@ public static void RunCore(IHost host, ExecuteParameters parameters)
                     InProcessDiagnoserHandler = compositeInProcessDiagnoserHandler
                 };
 
-                using (var engine = job
+                var results = job
                     .ResolveValue(InfrastructureMode.EngineFactoryCharacteristic, InfrastructureResolver.Instance)
-                    .CreateReadyToRun(engineParameters))
-                {
-                    var results = engine.Run();
+                    .Create(engineParameters)
+                    .Run();
+                host.ReportResults(results); // printing costs memory, do this after runs
 
-                    host.ReportResults(results); // printing costs memory, do this after runs
-                }
                 compositeInProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
             }
         }

tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs

@@ -50,40 +50,32 @@ public void Empty() { }
 
         public class CustomFactory : IEngineFactory
         {
-            public IEngine CreateReadyToRun(EngineParameters engineParameters)
-            {
-                var engine = new CustomEngine
-                {
-                    GlobalCleanupAction = engineParameters.GlobalCleanupAction,
-                    GlobalSetupAction = engineParameters.GlobalSetupAction
-                };
-
-                engine.GlobalSetupAction?.Invoke(); // engine factory is now supposed to create an engine which is ready to run (hence the method name change)
-
-                return engine;
-            }
+            public IEngine Create(EngineParameters engineParameters)
+                => new CustomEngine(engineParameters);
         }
 
-        public class CustomEngine : IEngine
+        public class CustomEngine(EngineParameters engineParameters) : IEngine
         {
             public RunResults Run()
             {
+                engineParameters.GlobalSetupAction.Invoke();
                 Console.WriteLine(EngineRunMessage);
-
-                return new RunResults(
-                    new List<Measurement>
-                    {
-                        new(1, IterationMode.Overhead, IterationStage.Actual, 1, 1, 1),
-                        new(1, IterationMode.Workload, IterationStage.Actual, 1, 1, 1)
-                    },
-                    OutlierMode.DontRemove,
-                    default);
+                try
+                {
+                    return new RunResults(
+                        [
+                            new(1, IterationMode.Overhead, IterationStage.Actual, 1, 1, 1),
+                            new(1, IterationMode.Workload, IterationStage.Actual, 1, 1, 1)
+                        ],
+                        OutlierMode.DontRemove,
+                        default
+                    );
+                }
+                finally
+                {
+                    engineParameters.GlobalCleanupAction.Invoke();
+                }
             }
-
-            public void Dispose() => GlobalCleanupAction?.Invoke();
-
-            public Action GlobalSetupAction { get; set; }
-            public Action GlobalCleanupAction { get; set; }
         }
     }
 }

tests/BenchmarkDotNet.IntegrationTests/InProcess.EmitTests/Runnable_0.cs

@@ -18,14 +18,10 @@ public static void Run(IHost host, ExecuteParameters parameters)
             if (job == null)
                 return;
 
-            using (var engine = engineFactory.CreateReadyToRun(engineParameters))
-            {
-                var results = engine.Run();
+            var results = engineFactory.Create(engineParameters).Run();
+            host.ReportResults(results); // printing costs memory, do this after runs
 
-                host.ReportResults(results); // printing costs memory, do this after runs
-
-                instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
-            }
+            instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
             engineParameters.InProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
         }
 

tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs

@@ -289,6 +289,7 @@ public void Setup()
             [GlobalCleanup]
             public void Cleanup()
             {
+                countdownEvent.Reset(ThreadsCount);
                 keepRunning = false;
                 barrier.SignalAndWait();
                 foreach (var thread in threads)