-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Issue #1024: Calculate baseline by the fastest benchmark #2171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
deedc64
fd9f34f
d20c117
d8d99e2
873243d
885bc04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| using BenchmarkDotNet.Configs; | ||
| using System; | ||
|
|
||
| namespace BenchmarkDotNet.Attributes | ||
| { | ||
| [AttributeUsage(AttributeTargets.Class)] | ||
| public class AutomaticBaselineAttribute : Attribute, IConfigSource | ||
| { | ||
| public IConfig Config { get; } | ||
|
|
||
| public AutomaticBaselineAttribute(AutomaticBaselineMode mode) => Config = ManualConfig.CreateEmpty().WithAutomaticBaseline(mode); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| namespace BenchmarkDotNet.Configs | ||
| { | ||
| public enum AutomaticBaselineMode | ||
| { | ||
| None, | ||
| Fastest | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ public class Summary | |
| private ImmutableDictionary<BenchmarkCase, BenchmarkReport> ReportMap { get; } | ||
| private BaseliningStrategy BaseliningStrategy { get; } | ||
| private bool? isMultipleRuntimes; | ||
| private readonly BenchmarkCase inferredBaselineBenchmarkCase; | ||
|
|
||
| public Summary( | ||
| string title, | ||
|
|
@@ -62,13 +63,32 @@ public Summary( | |
| DisplayPrecisionManager = new DisplayPrecisionManager(this); | ||
| Orderer = GetConfiguredOrdererOrDefaultOne(reports.Select(report => report.BenchmarkCase.Config)); | ||
| BenchmarksCases = Orderer.GetSummaryOrder(reports.Select(report => report.BenchmarkCase).ToImmutableArray(), this).ToImmutableArray(); // we sort it first | ||
| inferredBaselineBenchmarkCase = GetFastestBenchmarkCase(reports); | ||
| Reports = BenchmarksCases.Select(b => ReportMap[b]).ToImmutableArray(); // we use sorted collection to re-create reports list | ||
| BaseliningStrategy = BaseliningStrategy.Create(BenchmarksCases); | ||
| Style = GetConfiguredSummaryStyleOrDefaultOne(BenchmarksCases).WithCultureInfo(cultureInfo); | ||
| Table = GetTable(Style); | ||
| AllRuntimes = BuildAllRuntimes(HostEnvironmentInfo, Reports); | ||
| } | ||
|
|
||
| private static BenchmarkCase GetFastestBenchmarkCase(ImmutableArray<BenchmarkReport> reports) | ||
| { | ||
| if (reports.Any() && reports.All(r => r.BenchmarkCase.Config.AutomaticBaselineMode == AutomaticBaselineMode.Fastest)) | ||
| { | ||
| var fastestReport = reports.First(); | ||
| foreach (var report in reports.Skip(1)) | ||
| { | ||
| if (report.ResultStatistics.Mean < fastestReport.ResultStatistics.Mean) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I actually thought about it but decided to keep a new feature overriding more local configurations. Added a validation.
Could make sense but not to me. Ratio column is enough for me, the fastest is just a baseline (ratio == 1)
Fixed Thanks for your review Yegor 👍 |
||
| { | ||
| fastestReport = report; | ||
| } | ||
| } | ||
| return fastestReport.BenchmarkCase; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| [PublicAPI] public bool HasReport(BenchmarkCase benchmarkCase) => ReportMap.ContainsKey(benchmarkCase); | ||
|
|
||
| /// <summary> | ||
|
|
@@ -133,7 +153,11 @@ public string GetLogicalGroupKey(BenchmarkCase benchmarkCase) | |
| => Orderer.GetLogicalGroupKey(BenchmarksCases, benchmarkCase); | ||
|
|
||
| public bool IsBaseline(BenchmarkCase benchmarkCase) | ||
| => BaseliningStrategy.IsBaseline(benchmarkCase); | ||
| { | ||
| return inferredBaselineBenchmarkCase != null | ||
| ? inferredBaselineBenchmarkCase == benchmarkCase | ||
| : BaseliningStrategy.IsBaseline(benchmarkCase); | ||
| } | ||
|
|
||
| [CanBeNull] | ||
| public BenchmarkCase GetBaseline(string logicalGroupKey) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| using BenchmarkDotNet.Attributes; | ||
| using BenchmarkDotNet.Columns; | ||
| using BenchmarkDotNet.Configs; | ||
| using BenchmarkDotNet.Jobs; | ||
| using BenchmarkDotNet.Reports; | ||
| using BenchmarkDotNet.Running; | ||
| using System.Linq; | ||
| using System.Threading; | ||
| using Xunit; | ||
|
|
||
| namespace BenchmarkDotNet.IntegrationTests | ||
| { | ||
| public class AutomaticBaselineTests | ||
| { | ||
| [Fact] | ||
| public void AutomaticBaselineSelectionIsCorrect() | ||
| { | ||
| var config = CreateConfig(); | ||
|
|
||
| var summary = BenchmarkRunner.Run<BaselineSample>(); | ||
|
|
||
| var table = summary.GetTable(SummaryStyle.Default); | ||
| var column = table.Columns.Single(c => c.Header == "Ratio"); | ||
| Assert.Equal(2, column.Content.Length); | ||
| Assert.Equal(1.0, double.Parse(column.Content[1])); // Ratio of TwoMilliseconds | ||
| Assert.True(double.Parse(column.Content[0]) > 1.0); // Ratio of TwoHundredMilliseconds | ||
| } | ||
|
|
||
| [AutomaticBaseline(AutomaticBaselineMode.Fastest)] | ||
| public class BaselineSample | ||
| { | ||
| [Benchmark] | ||
| public void TwoHundredMilliseconds() | ||
| { | ||
| Thread.Sleep(200); | ||
| } | ||
|
|
||
| [Benchmark] | ||
| public void TwoMilliseconds() | ||
| { | ||
| Thread.Sleep(2); | ||
| } | ||
| } | ||
|
|
||
| private IConfig CreateConfig() | ||
| => ManualConfig.CreateEmpty() | ||
| .AddJob(Job.ShortRun | ||
| .WithEvaluateOverhead(false) // no need to run idle for this test | ||
| .WithWarmupCount(0) // don't run warmup to save some time for our CI runs | ||
| .WithIterationCount(1)) // single iteration is enough for us | ||
| .AddColumnProvider(DefaultColumnProviders.Instance); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correctly works when 2 benchmark classes have different
AutomaticBaselineMode?To join summaries, use:
--join(for cmd)BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).RunAllJoined()Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea itself works because the benchmarks are already grouped by the type so that Summary contains the cases of either C1 or C2 only. However, this particular example doesn't print the ratio for all the methods. I guess BDN trims columns that don't exist in all results (C1's benchmarks contain ration but C2's ones don't). If you set
AutomaticBaselineMode.Fastestfor C2, it prints correctly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ManualConfig.Union(...)resets theAutomaticBaselineMode:The problem is that the final config has only one value of
AutomaticBaselineMode, but it should have a different value for each benchmark class to allow:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not like that, the configs are different. I see that this inferredBaselineBenchmarkCase is calculated properly. There is probably a need to add a column or something like that, I will try to identify the problem.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is like that :)
Apply it first:
Sample
If we uncomment
AutomaticBaselineforMyClass1only:If we uncomment
AutomaticBaselineforMyClass2only (this is the correct table):Debug these lines to see how
IConfig.AutomaticBaselineModechanges:BenchmarkDotNet/src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Lines 106 to 107 in 6bb61ce
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, you are right. I didn't notice that Summary is built the third time (where you have all 4 methods). I was checking first two Summary instances (first two benchmarks, then second two). Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this is what I suggest we do d20c117
None) values to build configs.