From 3656a92bf11c064da8c1b5ba5272d97c662f6f56 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 27 May 2018 20:54:25 +0200 Subject: [PATCH] if IterationSetup is provided, and InvocationCount and UnrollFactor are not, run benchmark once per iteration to avoid user confusion, fixes #730 --- src/BenchmarkDotNet/Jobs/JobExtensions.cs | 21 +++++++--- src/BenchmarkDotNet/Running/Benchmark.cs | 5 ++- .../Running/BenchmarkConverter.cs | 2 +- .../Running/ClassicBenchmarkConverter.cs | 2 +- .../CodeGeneratorTests.cs | 2 +- .../Running/BenchmarkConverterTests.cs | 39 +++++++++++++++++++ .../Validators/CompilationValidatorTests.cs | 2 +- 7 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/BenchmarkDotNet/Jobs/JobExtensions.cs b/src/BenchmarkDotNet/Jobs/JobExtensions.cs index cdbd382b70..ce39c1884f 100644 --- a/src/BenchmarkDotNet/Jobs/JobExtensions.cs +++ b/src/BenchmarkDotNet/Jobs/JobExtensions.cs @@ -5,6 +5,7 @@ using BenchmarkDotNet.Engines; using BenchmarkDotNet.Environments; using BenchmarkDotNet.Horology; +using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Jobs { @@ -33,6 +34,7 @@ public static class JobExtensions public static Job WithIterationTime(this Job job, TimeInterval time) => job.WithCore(j => j.Run.IterationTime = time); public static Job WithInvocationCount(this Job job, int count) => job.WithCore(j => j.Run.InvocationCount = count); public static Job WithUnrollFactor(this Job job, int factor) => job.WithCore(j => j.Run.UnrollFactor = factor); + public static Job RunOncePerIteration(this Job job) => job.WithInvocationCount(1).WithUnrollFactor(1); public static Job WithMinTargetIterationCount(this Job job, int count) => job.WithCore(j => j.Run.MinTargetIterationCount = count); public static Job WithMaxTargetIterationCount(this Job job, int count) => job.WithCore(j => j.Run.MaxTargetIterationCount = count); @@ -56,12 +58,19 @@ public static class JobExtensions // Meta public static Job AsBaseline(this Job job) => job.WithCore(j => j.Meta.IsBaseline = true); public static Job WithIsBaseline(this Job job, bool value) => value ? job.AsBaseline() : job; - - // Info - [Obsolete] - public static string GetShortInfo(this Job job) => job.ResolvedId; - [Obsolete] - public static string GetFullInfo(this Job job) => CharacteristicSetPresenter.Default.ToPresentation(job); + + internal static Job MakeSettingsUserFriendly(this Job job, Target target) + { + // users expect that if IterationSetup is configured, it should be run before every benchmark invocation https://github.com/dotnet/BenchmarkDotNet/issues/730 + if (target.IterationSetupMethod != null + && !job.HasValue(RunMode.InvocationCountCharacteristic) + && !job.HasValue(RunMode.UnrollFactorCharacteristic)) + { + return job.RunOncePerIteration(); + } + + return job; + } private static Job WithCore(this Job job, Action updateCallback) { diff --git a/src/BenchmarkDotNet/Running/Benchmark.cs b/src/BenchmarkDotNet/Running/Benchmark.cs index f577c65fc0..0a640af231 100644 --- a/src/BenchmarkDotNet/Running/Benchmark.cs +++ b/src/BenchmarkDotNet/Running/Benchmark.cs @@ -16,7 +16,7 @@ public class Benchmark : IComparable public override string ToString() => DisplayInfo; - public Benchmark(Target target, Job job, ParameterInstances parameters) + private Benchmark(Target target, Job job, ParameterInstances parameters) { Target = target; Job = job; @@ -28,5 +28,8 @@ public Benchmark(Target target, Job job, ParameterInstances parameters) public bool IsBaseline() => Target.Baseline || Job.Meta.IsBaseline; public bool HasArguments => Parameters != null && Parameters.Items.Any(parameter => parameter.IsArgument); + + public static Benchmark Create(Target target, Job job, ParameterInstances parameters) + => new Benchmark(target, job.MakeSettingsUserFriendly(target), parameters); } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs index 6e0f9708f5..9dc4434877 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs @@ -66,7 +66,7 @@ private static BenchmarkRunInfo MethodsToBenchmarksWithFullConfig(Type containin from job in jobs from parameterInstance in parameterInstancesList from argumentDefinition in argumentsDefinitions - select new Benchmark(target, job, new ParameterInstances(parameterInstance.Items.Concat(argumentDefinition.Items).ToArray())) + select Benchmark.Create(target, job, new ParameterInstances(parameterInstance.Items.Concat(argumentDefinition.Items).ToArray())) ); } diff --git a/src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs b/src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs index c90b87b41e..1a7957841b 100644 --- a/src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs +++ b/src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs @@ -78,7 +78,7 @@ public static BenchmarkRunInfo[] SourceToBenchmarks(string source, IConfig confi var benchmarks = runInfo.Benchmarks.Select(b => { var target = b.Target; - return new Benchmark( + return Benchmark.Create( new Target(target.Type, target.Method, target.GlobalSetupMethod, target.GlobalCleanupMethod, target.IterationSetupMethod, target.IterationCleanupMethod, target.MethodDisplayInfo, benchmarkContent, target.Baseline, target.Categories, target.OperationsPerInvoke), diff --git a/tests/BenchmarkDotNet.Tests/CodeGeneratorTests.cs b/tests/BenchmarkDotNet.Tests/CodeGeneratorTests.cs index c3f3b544f1..954687f407 100644 --- a/tests/BenchmarkDotNet.Tests/CodeGeneratorTests.cs +++ b/tests/BenchmarkDotNet.Tests/CodeGeneratorTests.cs @@ -22,7 +22,7 @@ public void AsyncVoidIsNotSupported() .Single(method => method.Name == "AsyncVoidMethod"); var target = new Target(typeof(CodeGeneratorTests), asyncVoidMethod); - var benchmark = new Benchmark(target, Job.Default, null); + var benchmark = Benchmark.Create(target, Job.Default, null); Assert.Throws(() => CodeGenerator.Generate(new BuildPartition(new[] { new BenchmarkBuildInfo(benchmark, ManualConfig.CreateEmpty().AsReadOnly(), 0) }, BenchmarkRunner.DefaultResolver))); } diff --git a/tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs b/tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs index 1bb2d5bc4d..4070e8fe3c 100644 --- a/tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs +++ b/tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs @@ -1,5 +1,7 @@ using System.Linq; using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Running; using Xunit; @@ -70,5 +72,42 @@ public override void Cleanup() { } } + + [Fact] + public void IfIterationSetupIsProvidedTheBenchmarkShouldRunOncePerIteration() + { + var benchmark = BenchmarkConverter.TypeToBenchmarks(typeof(Derived)).Benchmarks.Single(); + + Assert.Equal(1, benchmark.Job.Run.InvocationCount); + Assert.Equal(1, benchmark.Job.Run.UnrollFactor); + } + + [Fact] + public void InvocationCountIsRespectedForBenchmarksWithIterationSetup() + { + const int InvocationCount = 100; + + var benchmark = BenchmarkConverter.TypeToBenchmarks(typeof(Derived), + DefaultConfig.Instance.With(Job.Default + .WithInvocationCount(InvocationCount))) + .Benchmarks.Single(); + + Assert.Equal(InvocationCount, benchmark.Job.Run.InvocationCount); + Assert.NotNull(benchmark.Target.IterationSetupMethod); + } + + [Fact] + public void UnrollFactorIsRespectedForBenchmarksWithIterationSetup() + { + const int UnrollFactor = 13; + + var benchmark = BenchmarkConverter.TypeToBenchmarks(typeof(Derived), + DefaultConfig.Instance.With(Job.Default + .WithUnrollFactor(UnrollFactor))) + .Benchmarks.Single(); + + Assert.Equal(UnrollFactor, benchmark.Job.Run.UnrollFactor); + Assert.NotNull(benchmark.Target.IterationSetupMethod); + } } } diff --git a/tests/BenchmarkDotNet.Tests/Validators/CompilationValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/CompilationValidatorTests.cs index 9e5713c2cb..04baf25353 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/CompilationValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/CompilationValidatorTests.cs @@ -20,7 +20,7 @@ public void BenchmarkedMethodNameMustNotContainWhitespaces() var parameters = new ValidationParameters( new[] { - new Benchmark( + Benchmark.Create( new Target( typeof(CompilationValidatorTests), method.Method),