-
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 all 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,97 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using BenchmarkDotNet.Attributes; | ||
using BenchmarkDotNet.Configs; | ||
using BenchmarkDotNet.Jobs; | ||
using BenchmarkDotNet.Loggers; | ||
using BenchmarkDotNet.Running; | ||
using Microsoft.ML.Runtime.Internal.CpuMath; | ||
using System; | ||
using System.Linq; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
namespace Microsoft.ML.Benchmarks.Tests | ||
{ | ||
public class TestConfig : RecommendedConfig | ||
{ | ||
protected override Job GetJobDefinition() => Job.Dry; // Job.Dry runs the benchmark just once | ||
} | ||
|
||
public class BenchmarkTouchingNativeDependency | ||
{ | ||
[Benchmark] | ||
public float Simple() => CpuMathUtils.Sum(Enumerable.Range(0, 1024).Select(Convert.ToSingle).ToArray(), 1024); | ||
} | ||
|
||
public class BenchmarksTest | ||
{ | ||
private const string SkipTheDebug = | ||
#if DEBUG | ||
"BenchmarkDotNet does not allow running the benchmarks in Debug, so this test is disabled for DEBUG"; | ||
#else | ||
""; | ||
#endif | ||
|
||
public BenchmarksTest(ITestOutputHelper output) => Output = output; | ||
|
||
private ITestOutputHelper Output { get; } | ||
|
||
[Fact(Skip = SkipTheDebug)] | ||
public void BenchmarksProjectIsNotBroken() | ||
{ | ||
var summary = BenchmarkRunner.Run<BenchmarkTouchingNativeDependency>(new TestConfig().With(new OutputLogger(Output))); | ||
|
||
Assert.False(summary.HasCriticalValidationErrors, "The \"Summary\" should have NOT \"HasCriticalValidationErrors\""); | ||
|
||
Assert.True(summary.Reports.Any(), "The \"Summary\" should contain at least one \"BenchmarkReport\" in the \"Reports\" collection"); | ||
|
||
Assert.True(summary.Reports.All(r => r.BuildResult.IsBuildSuccess), | ||
"The following benchmarks are failed to build: " + | ||
string.Join(", ", summary.Reports.Where(r => !r.BuildResult.IsBuildSuccess).Select(r => r.BenchmarkCase.DisplayInfo))); | ||
|
||
Assert.True(summary.Reports.All(r => r.ExecuteResults != null), | ||
"The following benchmarks don't have any execution results: " + | ||
string.Join(", ", summary.Reports.Where(r => r.ExecuteResults == null).Select(r => r.BenchmarkCase.DisplayInfo))); | ||
|
||
Assert.True(summary.Reports.All(r => r.ExecuteResults.Any(er => er.FoundExecutable && er.Data.Any())), | ||
"All reports should have at least one \"ExecuteResult\" with \"FoundExecutable\" = true and at least one \"Data\" item"); | ||
|
||
Assert.True(summary.Reports.All(report => report.AllMeasurements.Any()), | ||
"All reports should have at least one \"Measurement\" in the \"AllMeasurements\" collection"); | ||
} | ||
} | ||
|
||
public class OutputLogger : AccumulationLogger | ||
{ | ||
private readonly ITestOutputHelper testOutputHelper; | ||
private string currentLine = ""; | ||
|
||
public OutputLogger(ITestOutputHelper testOutputHelper) | ||
{ | ||
this.testOutputHelper = testOutputHelper ?? throw new ArgumentNullException(nameof(testOutputHelper)); | ||
} | ||
|
||
public override void Write(LogKind logKind, string text) | ||
{ | ||
currentLine += text; | ||
base.Write(logKind, text); | ||
} | ||
|
||
public override void WriteLine() | ||
{ | ||
testOutputHelper.WriteLine(currentLine); | ||
currentLine = ""; | ||
base.WriteLine(); | ||
} | ||
|
||
public override void WriteLine(LogKind logKind, string text) | ||
{ | ||
testOutputHelper.WriteLine(currentLine + text); | ||
currentLine = ""; | ||
base.WriteLine(logKind, text); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\Microsoft.ML.Benchmarks\Microsoft.ML.Benchmarks.csproj" /> | ||
|
||
<NativeAssemblyReference Include="CpuMathNative" /> | ||
</ItemGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
using BenchmarkDotNet.Configs; | ||
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); // this config contains all of the basic settings (exporters, columns etc) | ||
|
||
Add(GetJobDefinition() // job defines how many times given benchmark should be executed | ||
.With(CreateToolchain())); // toolchain is responsible for generating, building and running dedicated executable per benchmark | ||
|
||
Add(new ExtraMetricColumn()); // an extra colum that can display additional metric reported by the benchmarks | ||
|
||
UnionRule = ConfigUnionRule.AlwaysUseLocal; // global config can be overwritten with local (the one set via [ConfigAttribute]) | ||
} | ||
|
||
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), // custom generator that copies native dependencies | ||
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 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,16 +7,12 @@ | |
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
internal class Helpers | ||
{ | ||
public static string DatasetNotFound = "Could not find {0} Please ensure you have run 'build.cmd -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=true' from the root"; | ||
} | ||
|
||
// Adding this class to not print anything to the console. | ||
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 or @adamsitnik - FYI - whoever is in the benchmark tests again. A new class that just came in is the |
||
// This is required for the current version of BenchmarkDotNet | ||
internal class EmptyWriter : TextWriter | ||
{ | ||
internal static readonly EmptyWriter Instance = new EmptyWriter(); | ||
|
||
public override Encoding Encoding => null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using Microsoft.ML.Core.Data; | ||
using Microsoft.ML.Runtime; | ||
using Microsoft.ML.Runtime.Data; | ||
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
internal static class EnvironmentFactory | ||
{ | ||
internal static ConsoleEnvironment CreateClassificationEnvironment<TLoader, TTransformer, TTrainer>() | ||
where TLoader : IDataReader<IMultiStreamSource> | ||
where TTransformer : ITransformer | ||
where TTrainer : ITrainer | ||
{ | ||
var environment = new ConsoleEnvironment(verbose: false, sensitivity: MessageSensitivity.None, outWriter: EmptyWriter.Instance); | ||
|
||
environment.ComponentCatalog.RegisterAssembly(typeof(TLoader).Assembly); | ||
environment.ComponentCatalog.RegisterAssembly(typeof(TTransformer).Assembly); | ||
environment.ComponentCatalog.RegisterAssembly(typeof(TTrainer).Assembly); | ||
|
||
return environment; | ||
} | ||
|
||
internal static ConsoleEnvironment CreateRankingEnvironment<TEvaluator, TLoader, TTransformer, TTrainer>() | ||
where TEvaluator : IEvaluator | ||
where TLoader : IDataReader<IMultiStreamSource> | ||
where TTransformer : ITransformer | ||
where TTrainer : ITrainer | ||
{ | ||
var environment = new ConsoleEnvironment(verbose: false, sensitivity: MessageSensitivity.None, outWriter: EmptyWriter.Instance); | ||
|
||
environment.ComponentCatalog.RegisterAssembly(typeof(TEvaluator).Assembly); | ||
environment.ComponentCatalog.RegisterAssembly(typeof(TLoader).Assembly); | ||
environment.ComponentCatalog.RegisterAssembly(typeof(TTransformer).Assembly); | ||
environment.ComponentCatalog.RegisterAssembly(typeof(TTrainer).Assembly); | ||
|
||
environment.ComponentCatalog.RegisterAssembly(typeof(NAHandleTransform).Assembly); | ||
|
||
return environment; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
internal class Errors | ||
{ | ||
public static string DatasetNotFound = "Could not find {0} Please ensure you have run 'build.cmd -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=true' from the root"; | ||
} | ||
} |
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'm not sure I fully understand the goal of this test. It is just testing that copying native dependencies in a BDN project works correctly? It won't catch breaks in our actual benchmark tests, right?
I'm sort of wondering if this test is valuable going forward or not.... What kind of changes is it guarding against?
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 goal of this test is to make sure that we can build and run the benchmarks. So far they got broken few times, an example:
nuget.config
file was removed, BDN was ignoring theDirectory.Builds.props
file which contained the nuget feeds list and it was failing to restore one of the dependencies. Few people pinged me with the exact same problem.With this test I am confident that there will be no breaking changes for the benchmarks.
I would love to run the benchmarks as part of the test, however, they need a lot of time to execute. Some even 300s for single benchmark invocation