Skip to content

Commit 49dfa9e

Browse files
timcasselltimcassell
committed
Refactored Engine.Run to call GlobalSetup/Cleanup.
Removed `IDisposable` from `IEngine`. Renamed `IEngineFactory.CreateReadyToRun` back to `Create`. Updated engine tests.
1 parent 808dde5 commit 49dfa9e

File tree

10 files changed

+115
-217
lines changed

10 files changed

+115
-217
lines changed

‎src/BenchmarkDotNet/Engines/Engine.cs‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Diagnostics;
43
using System.Linq;
54
using System.Runtime.CompilerServices;
65
using System.Threading;
76
using BenchmarkDotNet.Characteristics;
8-
using BenchmarkDotNet.Environments;
9-
using BenchmarkDotNet.Helpers;
107
using BenchmarkDotNet.Jobs;
118
using BenchmarkDotNet.Portability;
129
using BenchmarkDotNet.Reports;
@@ -62,26 +59,37 @@ internal Engine(EngineParameters engineParameters)
6259
random = new Random(12345); // we are using constant seed to try to get repeatable results
6360
}
6461

65-
public void Dispose()
62+
public RunResults Run()
6663
{
64+
Parameters.GlobalSetupAction.Invoke();
65+
bool didThrow = false;
6766
try
6867
{
69-
Parameters.GlobalCleanupAction.Invoke();
68+
return RunCore();
7069
}
71-
catch (Exception e)
70+
catch
7271
{
73-
Host.SendError("Exception during GlobalCleanup!");
74-
Host.SendError(e.Message);
75-
76-
// we don't rethrow because this code is executed in a finally block
77-
// and it could possibly overwrite current exception #1045
72+
didThrow = true;
73+
throw;
74+
}
75+
finally
76+
{
77+
try
78+
{
79+
Parameters.GlobalCleanupAction.Invoke();
80+
}
81+
// We only catch if the benchmark threw to not overwrite the exception. #1045
82+
catch (Exception e) when (didThrow)
83+
{
84+
Host.SendError($"Exception during GlobalCleanup!{Environment.NewLine}{e}");
85+
}
7886
}
7987
}
8088

8189
// AggressiveOptimization forces the method to go straight to tier1 JIT, and will never be re-jitted,
8290
// eliminating tiered JIT as a potential variable in measurements.
8391
[MethodImpl(CodeGenHelper.AggressiveOptimizationOption)]
84-
public RunResults Run()
92+
private RunResults RunCore()
8593
{
8694
var measurements = new List<Measurement>();
8795

‎src/BenchmarkDotNet/Engines/EngineFactory.cs‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,7 @@
1-
namespace BenchmarkDotNet.Engines
2-
{
3-
public class EngineFactory : IEngineFactory
4-
{
5-
public IEngine CreateReadyToRun(EngineParameters engineParameters)
6-
{
7-
var engine = new Engine(engineParameters);
8-
9-
// TODO: Move GlobalSetup/Cleanup to Engine.Run.
10-
engine.Parameters.GlobalSetupAction.Invoke(); // whatever the settings are, we MUST call global setup here, the global cleanup is part of Engine's Dispose
1+
namespace BenchmarkDotNet.Engines;
112

12-
return engine;
13-
}
14-
}
15-
}
3+
public class EngineFactory : IEngineFactory
4+
{
5+
public IEngine Create(EngineParameters engineParameters)
6+
=> new Engine(engineParameters);
7+
}

‎src/BenchmarkDotNet/Engines/IEngine.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
1-
using System;
1+
namespace BenchmarkDotNet.Engines;
22

3-
namespace BenchmarkDotNet.Engines
3+
public interface IEngine
44
{
5-
public interface IEngine : IDisposable
6-
{
7-
RunResults Run();
8-
}
5+
RunResults Run();
96
}

‎src/BenchmarkDotNet/Engines/IEngineFactory.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ namespace BenchmarkDotNet.Engines
22
{
33
public interface IEngineFactory
44
{
5-
IEngine CreateReadyToRun(EngineParameters engineParameters);
5+
IEngine Create(EngineParameters engineParameters);
66
}
77
}

‎src/BenchmarkDotNet/Templates/BenchmarkType.txt‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,10 @@
5353
InProcessDiagnoserHandler = compositeInProcessDiagnoserHandler
5454
};
5555

56-
using (global::BenchmarkDotNet.Engines.IEngine engine = new $EngineFactoryType$().CreateReadyToRun(engineParameters))
57-
{
58-
global::BenchmarkDotNet.Engines.RunResults results = engine.Run();
59-
60-
host.ReportResults(results); // printing costs memory, do this after runs
56+
global::BenchmarkDotNet.Engines.RunResults results = new $EngineFactoryType$().Create(engineParameters).Run();
57+
host.ReportResults(results); // printing costs memory, do this after runs
6158

62-
instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
63-
}
59+
instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
6460
compositeInProcessDiagnoserHandler.Handle(global::BenchmarkDotNet.Engines.BenchmarkSignal.AfterEngine);
6561
}
6662

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

Lines changed: 34 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ class [BenchmarkDotNet]BenchmarkDotNet.Toolchains.Parameters.ExecuteParameters p
873873
var argsExceptInstance = prepareForRunMethodTemplate
874874
.GetParameters()
875875
.Skip(1)
876-
.Select(p => (ParameterInfo)new EmitParameterInfo(p.Position - 1, p.Name, p.ParameterType, p.Attributes, null))
876+
.Select(p => (ParameterInfo) new EmitParameterInfo(p.Position - 1, p.Name, p.ParameterType, p.Attributes, null))
877877
.ToArray();
878878
var methodBuilder = runnableBuilder.DefineStaticMethod(
879879
RunMethodName,
@@ -892,15 +892,13 @@ .locals init (
892892
[1] class [BenchmarkDotNet]BenchmarkDotNet.Jobs.Job,
893893
[2] class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters,
894894
[3] class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory,
895-
[4] class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine,
896-
[5] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
895+
[4] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
897896
)
898897
*/
899898
var instanceLocal = ilBuilder.DeclareLocal(runnableBuilder);
900899
var jobLocal = ilBuilder.DeclareLocal(typeof(Job));
901900
var engineParametersLocal = ilBuilder.DeclareLocal(typeof(EngineParameters));
902901
var engineFactoryLocal = ilBuilder.DeclareLocal(typeof(IEngineFactory));
903-
var engineLocal = ilBuilder.DeclareLocal(typeof(IEngine));
904902
var runResultsLocal = ilBuilder.DeclareLocal(typeof(RunResults));
905903

906904
/*
@@ -961,82 +959,48 @@ [5] valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults
961959
ilBuilder.EmitVoidReturn(methodBuilder);
962960

963961
/*
964-
// using (IEngine engine = engineFactory.CreateReadyToRun(engineParameters))
962+
// RunResults results = engineFactory.Create(engineParameters).Run();
965963
IL_0026: ldloc.3
966964
IL_0027: ldloc.2
967-
IL_0028: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory::CreateReadyToRun(class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters)
968-
IL_002d: stloc.s 4
965+
IL_0028: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngineFactory::Create(class [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters)
966+
IL_002d: callvirt instance valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine::Run()
967+
IL_0032: stloc.s 4
969968
*/
970-
var createReadyToRunMethod = typeof(IEngineFactory).GetMethod(nameof(IEngineFactory.CreateReadyToRun))
971-
?? throw new MissingMemberException(nameof(IEngineFactory.CreateReadyToRun));
969+
var createReadyToRunMethod = typeof(IEngineFactory).GetMethod(nameof(IEngineFactory.Create))
970+
?? throw new MissingMemberException(nameof(IEngineFactory.Create));
971+
var runMethodImpl = typeof(IEngine).GetMethod(nameof(IEngine.Run))
972+
?? throw new MissingMemberException(nameof(IEngine.Run));
972973
ilBuilder.MarkLabel(notNullLabel);
973974
ilBuilder.EmitLdloc(engineFactoryLocal);
974975
ilBuilder.EmitLdloc(engineParametersLocal);
975976
ilBuilder.Emit(OpCodes.Callvirt, createReadyToRunMethod);
976-
ilBuilder.EmitStloc(engineLocal);
977-
978-
// .try
979-
// {
980-
ilBuilder.BeginExceptionBlock();
981-
{
982-
/*
983-
// RunResults results = engine.Run();
984-
IL_002f: ldloc.s 4
985-
IL_0031: callvirt instance valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults [BenchmarkDotNet]BenchmarkDotNet.Engines.IEngine::Run()
986-
IL_0036: stloc.s 5
987-
*/
988-
var runMethodImpl = typeof(IEngine).GetMethod(nameof(IEngine.Run))
989-
?? throw new MissingMemberException(nameof(IEngine.Run));
990-
ilBuilder.EmitLdloc(engineLocal);
991-
ilBuilder.Emit(OpCodes.Callvirt, runMethodImpl);
992-
ilBuilder.EmitStloc(runResultsLocal);
993-
/*
994-
// host.ReportResults(results);
995-
IL_0038: ldarg.1
996-
IL_0039: ldloc.s 5
997-
IL_003b: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Engines.IHost::ReportResults(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults)
998-
*/
999-
var reportResultsMethod = typeof(IHost).GetMethod(nameof(IHost.ReportResults))
1000-
?? throw new MissingMemberException(nameof(IHost.ReportResults));
1001-
ilBuilder.EmitLdarg(hostArg);
1002-
ilBuilder.EmitLdloc(runResultsLocal);
1003-
ilBuilder.Emit(OpCodes.Callvirt, reportResultsMethod);
1004-
/*
1005-
// instance.__TrickTheJIT__();
1006-
IL_0040: ldloc.0
1007-
IL_0041: callvirt instance void BenchmarkDotNet.Autogenerated.ReplaceMe.Runnable0::__TrickTheJIT__()
1008-
*/
1009-
ilBuilder.Emit(OpCodes.Ldloc_0);
1010-
ilBuilder.Emit(OpCodes.Callvirt, trickTheJitMethod);
1011-
}
1012-
// finally
1013-
// {
1014-
ilBuilder.BeginFinallyBlock();
1015-
{
1016-
/*
1017-
IL_0048: ldloc.s 4
1018-
IL_004a: brfalse.s IL_0053
1019-
IL_004c: ldloc.s 4
1020-
IL_004e: callvirt instance void [mscorlib]System.IDisposable::Dispose()
1021-
*/
1022-
var disposeMethod = typeof(IDisposable).GetMethod(nameof(IDisposable.Dispose))
1023-
?? throw new MissingMemberException(nameof(IDisposable.Dispose));
1024-
var disposeNullLabel = ilBuilder.DefineLabel();
1025-
ilBuilder.EmitLdloc(engineLocal);
1026-
ilBuilder.Emit(OpCodes.Brfalse_S, disposeNullLabel);
1027-
ilBuilder.EmitLdloc(engineLocal);
1028-
ilBuilder.Emit(OpCodes.Callvirt, disposeMethod);
1029-
1030-
ilBuilder.MarkLabel(disposeNullLabel);
1031-
ilBuilder.EndExceptionBlock();
1032-
}
977+
ilBuilder.Emit(OpCodes.Callvirt, runMethodImpl);
978+
ilBuilder.EmitStloc(runResultsLocal);
979+
/*
980+
// host.ReportResults(runResults);
981+
IL_0034: ldarg.0
982+
IL_0035: ldloc.s 4
983+
IL_0037: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Engines.IHost::ReportResults(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.RunResults)
984+
*/
1033985

986+
var reportResultsMethod = typeof(IHost).GetMethod(nameof(IHost.ReportResults))
987+
?? throw new MissingMemberException(nameof(IHost.ReportResults));
988+
ilBuilder.EmitLdarg(hostArg);
989+
ilBuilder.EmitLdloc(runResultsLocal);
990+
ilBuilder.Emit(OpCodes.Callvirt, reportResultsMethod);
991+
/*
992+
// runnable_.__TrickTheJIT__();
993+
IL_003c: ldloc.0
994+
IL_003d: callvirt instance void BenchmarkDotNet.Autogenerated.ReplaceMe.Runnable_0::__TrickTheJIT__()
995+
*/
996+
ilBuilder.Emit(OpCodes.Ldloc_0);
997+
ilBuilder.Emit(OpCodes.Callvirt, trickTheJitMethod);
1034998
/*
1035999
// engineParameters.InProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
1036-
IL_0054: ldloc.2
1037-
IL_0055: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters::get_InProcessDiagnoserHandler()
1038-
IL_005a: ldc.i4.3
1039-
IL_005b: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler::Handle(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.BenchmarkSignal)
1000+
IL_0042: ldloc.2
1001+
IL_0043: callvirt instance class [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler [BenchmarkDotNet]BenchmarkDotNet.Engines.EngineParameters::get_InProcessDiagnoserHandler()
1002+
IL_0048: ldc.i4.5
1003+
IL_0049: callvirt instance void [BenchmarkDotNet]BenchmarkDotNet.Diagnosers.CompositeInProcessDiagnoserHandler::Handle(valuetype [BenchmarkDotNet]BenchmarkDotNet.Engines.BenchmarkSignal)
10401004
*/
10411005
ilBuilder.EmitLdloc(engineParametersLocal);
10421006
ilBuilder.Emit(OpCodes.Callvirt, typeof(EngineParameters).GetProperty(nameof(EngineParameters.InProcessDiagnoserHandler)).GetGetMethod());

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,12 @@ public static void RunCore(IHost host, ExecuteParameters parameters)
166166
InProcessDiagnoserHandler = compositeInProcessDiagnoserHandler
167167
};
168168

169-
using (var engine = job
169+
var results = job
170170
.ResolveValue(InfrastructureMode.EngineFactoryCharacteristic, InfrastructureResolver.Instance)
171-
.CreateReadyToRun(engineParameters))
172-
{
173-
var results = engine.Run();
171+
.Create(engineParameters)
172+
.Run();
173+
host.ReportResults(results); // printing costs memory, do this after runs
174174

175-
host.ReportResults(results); // printing costs memory, do this after runs
176-
}
177175
compositeInProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
178176
}
179177
}

‎tests/BenchmarkDotNet.IntegrationTests/CustomEngineTests.cs‎

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -50,40 +50,32 @@ public void Empty() { }
5050

5151
public class CustomFactory : IEngineFactory
5252
{
53-
public IEngine CreateReadyToRun(EngineParameters engineParameters)
54-
{
55-
var engine = new CustomEngine
56-
{
57-
GlobalCleanupAction = engineParameters.GlobalCleanupAction,
58-
GlobalSetupAction = engineParameters.GlobalSetupAction
59-
};
60-
61-
engine.GlobalSetupAction?.Invoke(); // engine factory is now supposed to create an engine which is ready to run (hence the method name change)
62-
63-
return engine;
64-
}
53+
public IEngine Create(EngineParameters engineParameters)
54+
=> new CustomEngine(engineParameters);
6555
}
6656

67-
public class CustomEngine : IEngine
57+
public class CustomEngine(EngineParameters engineParameters) : IEngine
6858
{
6959
public RunResults Run()
7060
{
61+
engineParameters.GlobalSetupAction.Invoke();
7162
Console.WriteLine(EngineRunMessage);
72-
73-
return new RunResults(
74-
new List<Measurement>
75-
{
76-
new(1, IterationMode.Overhead, IterationStage.Actual, 1, 1, 1),
77-
new(1, IterationMode.Workload, IterationStage.Actual, 1, 1, 1)
78-
},
79-
OutlierMode.DontRemove,
80-
default);
63+
try
64+
{
65+
return new RunResults(
66+
[
67+
new(1, IterationMode.Overhead, IterationStage.Actual, 1, 1, 1),
68+
new(1, IterationMode.Workload, IterationStage.Actual, 1, 1, 1)
69+
],
70+
OutlierMode.DontRemove,
71+
default
72+
);
73+
}
74+
finally
75+
{
76+
engineParameters.GlobalCleanupAction.Invoke();
77+
}
8178
}
82-
83-
public void Dispose() => GlobalCleanupAction?.Invoke();
84-
85-
public Action GlobalSetupAction { get; set; }
86-
public Action GlobalCleanupAction { get; set; }
8779
}
8880
}
8981
}

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,10 @@ public static void Run(IHost host, ExecuteParameters parameters)
1818
if (job == null)
1919
return;
2020

21-
using (var engine = engineFactory.CreateReadyToRun(engineParameters))
22-
{
23-
var results = engine.Run();
21+
var results = engineFactory.Create(engineParameters).Run();
22+
host.ReportResults(results); // printing costs memory, do this after runs
2423

25-
host.ReportResults(results); // printing costs memory, do this after runs
26-
27-
instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
28-
}
24+
instance.__TrickTheJIT__(); // compile the method for disassembler, but without actual run of the benchmark ;)
2925
engineParameters.InProcessDiagnoserHandler.Handle(BenchmarkSignal.AfterEngine);
3026
}
3127

0 commit comments

Comments
 (0)