Skip to content

Commit

Permalink
if IterationSetup is provided, and InvocationCount and UnrollFactor a…
Browse files Browse the repository at this point in the history
…re not, run benchmark once per iteration to avoid user confusion, fixes dotnet#730
  • Loading branch information
adamsitnik authored and alinasmirnova committed Sep 22, 2018
1 parent f211c51 commit 3656a92
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 11 deletions.
21 changes: 15 additions & 6 deletions src/BenchmarkDotNet/Jobs/JobExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using BenchmarkDotNet.Engines;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Horology;
using BenchmarkDotNet.Running;

namespace BenchmarkDotNet.Jobs
{
Expand Down Expand Up @@ -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);

Expand All @@ -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<Job> updateCallback)
{
Expand Down
5 changes: 4 additions & 1 deletion src/BenchmarkDotNet/Running/Benchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class Benchmark : IComparable<Benchmark>

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;
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion tests/BenchmarkDotNet.Tests/CodeGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NotSupportedException>(() => CodeGenerator.Generate(new BuildPartition(new[] { new BenchmarkBuildInfo(benchmark, ManualConfig.CreateEmpty().AsReadOnly(), 0) }, BenchmarkRunner.DefaultResolver)));
}
Expand Down
39 changes: 39 additions & 0 deletions tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.Linq;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using Xunit;

Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void BenchmarkedMethodNameMustNotContainWhitespaces()
var parameters = new ValidationParameters(
new[]
{
new Benchmark(
Benchmark.Create(
new Target(
typeof(CompilationValidatorTests),
method.Method),
Expand Down

0 comments on commit 3656a92

Please sign in to comment.