-
-
Notifications
You must be signed in to change notification settings - Fork 992
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
Open
Serg046
wants to merge
6
commits into
dotnet:master
Choose a base branch
from
Serg046:issue-1024
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
deedc64
Issue #1024: Calculate baseline by the fastest benchmark
Serg046 fd9f34f
Issue #1024: Add validations
Serg046 d20c117
Issue #1024: Correct configuration
Serg046 d8d99e2
Issue #1024: Make AutomaticBaselineSelectionIsCorrect test stable enough
Serg046 873243d
Update src/BenchmarkDotNet/Reports/Summary.cs
Serg046 885bc04
Fix a typo
Serg046 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
13 changes: 13 additions & 0 deletions
13
src/BenchmarkDotNet/Attributes/AutomaticBaselineAttribute.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
namespace BenchmarkDotNet.Configs | ||
{ | ||
public enum AutomaticBaselineMode | ||
{ | ||
None, | ||
Fastest | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
tests/BenchmarkDotNet.IntegrationTests/AutomaticBaselineTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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()
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.Fastest
for 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.
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
AutomaticBaseline
forMyClass1
only:If we uncomment
AutomaticBaseline
forMyClass2
only (this is the correct table):Debug these lines to see how
IConfig.AutomaticBaselineMode
changes:BenchmarkDotNet/src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Lines 106 to 107 in 6bb61ce
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.