-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix the benchmarks #1001
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
Fix the benchmarks #1001
Changes from 4 commits
9c5b40a
05b7d90
bf54bb9
0ce2640
c1fb2c0
41fb025
aa45f38
bf3a85a
b555773
c1eb626
10bd93b
c340cdf
4112eab
9fbe544
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,9 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<configuration> | ||
<packageSources> | ||
<!--To inherit the global NuGet package sources remove the <clear/> line below --> | ||
<clear /> | ||
<add key="nuget" value="https://api.nuget.org/v3/index.json" /> | ||
<add key="net core" value=" https://dotnet.myget.org/F/dotnet-core/api/v3/index.json" /> | ||
</packageSources> | ||
</configuration> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
using BenchmarkDotNet.Configs; | ||
using BenchmarkDotNet.Diagnosers; | ||
using BenchmarkDotNet.Jobs; | ||
using BenchmarkDotNet.Toolchains; | ||
using BenchmarkDotNet.Toolchains.CsProj; | ||
using BenchmarkDotNet.Toolchains.DotNetCli; | ||
using Microsoft.ML.Benchmarks.Harness; | ||
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
public class RecommendedConfig : ManualConfig | ||
{ | ||
public RecommendedConfig() | ||
{ | ||
Add(DefaultConfig.Instance | ||
.With(GetJobDefinition().With(CreateToolchain())) | ||
.With(new ExtraMetricColumn()) | ||
.With(MemoryDiagnoser.Default)); | ||
|
||
UnionRule = ConfigUnionRule.AlwaysUseLocal; | ||
} | ||
|
||
protected virtual Job GetJobDefinition() | ||
=> Job.Default | ||
.WithWarmupCount(1) // ML.NET benchmarks are typically CPU-heavy benchmarks, 1 warmup is usually enough | ||
.WithMaxIterationCount(20); | ||
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. @adamsitnik I think we should make the default configuration as the train config as most of the tests are running in the train configuration ? 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. @Anipik thank you for the suggestion! The |
||
|
||
/// <summary> | ||
/// we need our own toolchain because MSBuild by default does not copy recursive native dependencies to the output | ||
/// </summary> | ||
private IToolchain CreateToolchain() | ||
{ | ||
var csProj = CsProjCoreToolchain.Current.Value; | ||
var tfm = NetCoreAppSettings.Current.Value.TargetFrameworkMoniker; | ||
|
||
return new Toolchain( | ||
tfm, | ||
new ProjectGenerator(tfm), | ||
csProj.Builder, | ||
csProj.Executor); | ||
} | ||
} | ||
|
||
public class TrainConfig : RecommendedConfig | ||
{ | ||
protected override Job GetJobDefinition() | ||
=> Job.Dry // the "Dry" job runs the benchmark exactly once, without any warmup to mimic real-world scenario | ||
.WithLaunchCount(3); // BDN will run 3 dedicated processes, sequentially | ||
} | ||
} |
This file was deleted.
This file was deleted.
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.
@eerhardt will this affect anything else, or will they all keep using the MSBuild source?
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.
I'd like to not have this list duplicated. NuGet.configs are not great because they cannot be modified (either adding new sources or removing sources, or changing locations) without actually modifying the file.
Instead, would it be possible to:
RestoreSources
property out ofDirectory.Build.props
and put it in a separate file, saybuild\NuGetSources.props
.Directory.Build.props
imports that file.build\NuGetSources.props
file.That way we don't need to duplicate this list.
In reply to: 220023911 [](ancestors = 220023911)
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.
@eerhardt I have changed the way it works: now our project generator generates a simple .csproj file which includes the native dependencies and does not ignore
Directory.Build.props
file so BDN is using the nuget feeds defined inDirectory.Build.props