Skip to content

Commit 28bf214

Browse files
emanuel-v-remanuelra-tradamsitnikYegorStepanov
authored
adding validation errors when the benchmarks are unsupported (#2148)
* Refactoring tool chains in order to return validation errors * Replace IsSupported method by Validate which returns the errors instead of only a bool * Extract printing logic from tool chains * Remove logger dependency from IToolChain Co-authored-by: emanuelra-tr <emanuelra@etoro.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: Yegor Stepanov <yegor.stepanov@outlook.com>
1 parent 58d4bae commit 28bf214

File tree

22 files changed

+289
-207
lines changed

22 files changed

+289
-207
lines changed

‎samples/BenchmarkDotNet.Samples/IntroWasm.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public static void Run()
3939
NetCoreAppSettings netCoreAppSettings = new NetCoreAppSettings(
4040
targetFrameworkMoniker: "net5.0", runtimeFrameworkVersion: null, name: "Wasm",
4141
customDotNetCliPath: cliPath);
42-
IToolchain toolChain = WasmToolChain.From(netCoreAppSettings);
42+
IToolchain toolChain = WasmToolchain.From(netCoreAppSettings);
4343

4444
BenchmarkRunner.Run<IntroCustomMonoFluentConfig>(DefaultConfig.Instance
4545
.AddJob(Job.ShortRun.WithRuntime(runtime).WithToolchain(toolChain)));

‎src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
using BenchmarkDotNet.Loggers;
1818
using BenchmarkDotNet.Portability;
1919
using BenchmarkDotNet.Reports;
20-
using BenchmarkDotNet.Toolchains.NativeAot;
2120
using BenchmarkDotNet.Toolchains.CoreRun;
2221
using BenchmarkDotNet.Toolchains.CsProj;
2322
using BenchmarkDotNet.Toolchains.DotNetCli;
2423
using BenchmarkDotNet.Toolchains.InProcess.Emit;
25-
using BenchmarkDotNet.Toolchains.MonoWasm;
2624
using BenchmarkDotNet.Toolchains.MonoAotLLVM;
25+
using BenchmarkDotNet.Toolchains.MonoWasm;
26+
using BenchmarkDotNet.Toolchains.NativeAot;
2727
using CommandLine;
2828
using Perfolizer.Horology;
2929
using Perfolizer.Mathematics.OutlierDetection;
@@ -113,7 +113,7 @@ private static bool Validate(CommandLineOptions options, ILogger logger)
113113
}
114114
else if (runtimeMoniker == RuntimeMoniker.MonoAOTLLVM && (options.AOTCompilerPath == null || options.AOTCompilerPath.IsNotNullButDoesNotExist()))
115115
{
116-
logger.WriteLineError($"The provided {nameof(options.AOTCompilerPath)} \"{ options.AOTCompilerPath }\" does NOT exist. It MUST be provided.");
116+
logger.WriteLineError($"The provided {nameof(options.AOTCompilerPath)} \"{options.AOTCompilerPath}\" does NOT exist. It MUST be provided.");
117117
}
118118
}
119119

@@ -265,7 +265,7 @@ private static Job GetBaseJob(CommandLineOptions options, IConfig globalConfig)
265265
baseJob = baseJob.WithOutlierMode(options.Outliers);
266266

267267
if (options.Affinity.HasValue)
268-
baseJob = baseJob.WithAffinity((IntPtr) options.Affinity.Value);
268+
baseJob = baseJob.WithAffinity((IntPtr)options.Affinity.Value);
269269

270270
if (options.LaunchCount.HasValue)
271271
baseJob = baseJob.WithLaunchCount(options.LaunchCount.Value);
@@ -376,6 +376,7 @@ private static Job CreateJobForGivenRuntime(Job baseJob, string runtimeId, Comma
376376
return baseJob
377377
.WithRuntime(runtimeMoniker.GetRuntime())
378378
.WithToolchain(CsProjClassicNetToolchain.From(runtimeId, options.RestorePath?.FullName));
379+
379380
case RuntimeMoniker.NetCoreApp20:
380381
case RuntimeMoniker.NetCoreApp21:
381382
case RuntimeMoniker.NetCoreApp22:
@@ -390,26 +391,37 @@ private static Job CreateJobForGivenRuntime(Job baseJob, string runtimeId, Comma
390391
return baseJob
391392
.WithRuntime(runtimeMoniker.GetRuntime())
392393
.WithToolchain(CsProjCoreToolchain.From(new NetCoreAppSettings(runtimeId, null, runtimeId, options.CliPath?.FullName, options.RestorePath?.FullName)));
394+
393395
case RuntimeMoniker.Mono:
394396
return baseJob.WithRuntime(new MonoRuntime("Mono", options.MonoPath?.FullName));
397+
395398
case RuntimeMoniker.NativeAot60:
396399
return CreateAotJob(baseJob, options, runtimeMoniker, "6.0.0-*", "https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-experimental/nuget/v3/index.json");
400+
397401
case RuntimeMoniker.NativeAot70:
398402
return CreateAotJob(baseJob, options, runtimeMoniker, "", "https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7/nuget/v3/index.json");
403+
399404
case RuntimeMoniker.Wasm:
400405
return MakeWasmJob(baseJob, options, RuntimeInformation.IsNetCore ? CoreRuntime.GetCurrentVersion().MsBuildMoniker : "net5.0", runtimeMoniker);
406+
401407
case RuntimeMoniker.WasmNet50:
402408
return MakeWasmJob(baseJob, options, "net5.0", runtimeMoniker);
409+
403410
case RuntimeMoniker.WasmNet60:
404411
return MakeWasmJob(baseJob, options, "net6.0", runtimeMoniker);
412+
405413
case RuntimeMoniker.WasmNet70:
406414
return MakeWasmJob(baseJob, options, "net7.0", runtimeMoniker);
415+
407416
case RuntimeMoniker.MonoAOTLLVM:
408417
return MakeMonoAOTLLVMJob(baseJob, options, RuntimeInformation.IsNetCore ? CoreRuntime.GetCurrentVersion().MsBuildMoniker : "net6.0");
418+
409419
case RuntimeMoniker.MonoAOTLLVMNet60:
410420
return MakeMonoAOTLLVMJob(baseJob, options, "net6.0");
421+
411422
case RuntimeMoniker.MonoAOTLLVMNet70:
412423
return MakeMonoAOTLLVMJob(baseJob, options, "net7.0");
424+
413425
default:
414426
throw new NotSupportedException($"Runtime {runtimeId} is not supported");
415427
}
@@ -467,7 +479,7 @@ private static Job MakeWasmJob(Job baseJob, CommandLineOptions options, string m
467479
wasmDataDir: options.WasmDataDirectory?.FullName,
468480
moniker: moniker);
469481

470-
var toolChain = WasmToolChain.From(new NetCoreAppSettings(
482+
var toolChain = WasmToolchain.From(new NetCoreAppSettings(
471483
targetFrameworkMoniker: wasmRuntime.MsBuildMoniker,
472484
runtimeFrameworkVersion: null,
473485
name: wasmRuntime.Name,
@@ -555,7 +567,6 @@ private static string GetCoreRunToolchainDisplayName(IReadOnlyList<FileInfo> pat
555567
}
556568
}
557569

558-
559570
if (commonLongestPrefixIndex <= 1)
560571
return coreRunPath.FullName;
561572

@@ -573,4 +584,4 @@ private static bool TryParse(string runtime, out RuntimeMoniker runtimeMoniker)
573584
: Enum.TryParse<RuntimeMoniker>(runtime.Substring(0, index).Replace(".", string.Empty), ignoreCase: true, out runtimeMoniker);
574585
}
575586
}
576-
}
587+
}

‎src/BenchmarkDotNet/Loggers/LoggerExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public static class LoggerExtensions
2020

2121
public static void WriteLineHint(this ILogger logger, string text) => logger.WriteLine(LogKind.Hint, text);
2222

23-
public static void Write(this ILogger logger, string text) => logger.Write(LogKind.Default, text);
23+
public static void Write(this ILogger logger, string text) => logger.Write(LogKind.Default, text);
2424

2525
[PublicAPI]
2626
public static void WriteHelp(this ILogger logger, string text) => logger.Write(LogKind.Help, text);

‎src/BenchmarkDotNet/Reports/Summary.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,8 @@ public Summary(
8383
public bool IsMultipleRuntimes
8484
=> isMultipleRuntimes ??= BenchmarksCases.Length > 1 ? BenchmarksCases.Select(benchmark => benchmark.GetRuntime()).Distinct().Count() > 1 : false;
8585

86-
internal static Summary NothingToRun(string title, string resultsDirectoryPath, string logFilePath)
87-
=> new Summary(title, ImmutableArray<BenchmarkReport>.Empty, HostEnvironmentInfo.GetCurrent(), resultsDirectoryPath, logFilePath, TimeSpan.Zero, DefaultCultureInfo.Instance, ImmutableArray<ValidationError>.Empty, ImmutableArray<IColumnHidingRule>.Empty);
88-
89-
internal static Summary ValidationFailed(string title, string resultsDirectoryPath, string logFilePath, ImmutableArray<ValidationError> validationErrors)
90-
=> new Summary(title, ImmutableArray<BenchmarkReport>.Empty, HostEnvironmentInfo.GetCurrent(), resultsDirectoryPath, logFilePath, TimeSpan.Zero, DefaultCultureInfo.Instance, validationErrors, ImmutableArray<IColumnHidingRule>.Empty);
86+
internal static Summary ValidationFailed(string title, string resultsDirectoryPath, string logFilePath, ImmutableArray<ValidationError>? validationErrors = null)
87+
=> new Summary(title, ImmutableArray<BenchmarkReport>.Empty, HostEnvironmentInfo.GetCurrent(), resultsDirectoryPath, logFilePath, TimeSpan.Zero, DefaultCultureInfo.Instance, validationErrors ?? ImmutableArray<ValidationError>.Empty, ImmutableArray<IColumnHidingRule>.Empty);
9188

9289
internal static Summary Join(List<Summary> summaries, ClockSpan clockSpan)
9390
=> new Summary(
@@ -169,4 +166,4 @@ private static SummaryStyle GetConfiguredSummaryStyleOrDefaultOne(ImmutableArray
169166
.SingleOrDefault()
170167
?? SummaryStyle.Default;
171168
}
172-
}
169+
}

‎src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,19 @@ internal static Summary[] Run(BenchmarkRunInfo[] benchmarkRunInfos)
4747
{
4848
var compositeLogger = CreateCompositeLogger(benchmarkRunInfos, streamLogger);
4949

50-
var supportedBenchmarks = GetSupportedBenchmarks(benchmarkRunInfos, compositeLogger, resolver);
51-
if (!supportedBenchmarks.Any(benchmarks => benchmarks.BenchmarksCases.Any()))
52-
return new[] { Summary.NothingToRun(title, resultsFolderPath, logFilePath) };
50+
compositeLogger.WriteLineInfo("// Validating benchmarks:");
51+
52+
var (supportedBenchmarks, validationErrors) = GetSupportedBenchmarks(benchmarkRunInfos, compositeLogger, resolver);
53+
54+
validationErrors.AddRange(Validate(supportedBenchmarks));
55+
56+
PrintValidationErrors(compositeLogger, validationErrors);
5357

54-
var validationErrors = Validate(supportedBenchmarks, compositeLogger);
5558
if (validationErrors.Any(validationError => validationError.IsCritical))
56-
return new[] { Summary.ValidationFailed(title, resultsFolderPath, logFilePath, validationErrors) };
59+
return new[] { Summary.ValidationFailed(title, resultsFolderPath, logFilePath, validationErrors.ToImmutableArray()) };
60+
61+
if (!supportedBenchmarks.Any(benchmarks => benchmarks.BenchmarksCases.Any()))
62+
return new[] { Summary.ValidationFailed(title, resultsFolderPath, logFilePath) };
5763

5864
int totalBenchmarkCount = supportedBenchmarks.Sum(benchmarkInfo => benchmarkInfo.BenchmarksCases.Length);
5965
int benchmarksToRunCount = totalBenchmarkCount;
@@ -145,7 +151,7 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
145151
var cultureInfo = config.CultureInfo ?? DefaultCultureInfo.Instance;
146152
var reports = new List<BenchmarkReport>();
147153
string title = GetTitle(new[] { benchmarkRunInfo });
148-
var consoleTitle = RuntimeInformation.IsWindows() ? Console.Title : string.Empty;
154+
var consoleTitle = RuntimeInformation.IsWindows() ? Console.Title : string.Empty;
149155

150156
logger.WriteLineInfo($"// Found {benchmarks.Length} benchmarks:");
151157
foreach (var benchmark in benchmarks)
@@ -236,7 +242,7 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
236242
logFilePath,
237243
runEnd.GetTimeSpan() - runStart.GetTimeSpan(),
238244
cultureInfo,
239-
Validate(new[] { benchmarkRunInfo }, NullLogger.Instance), // validate them once again, but don't print the output
245+
Validate(benchmarkRunInfo), // validate them once again, but don't print the output
240246
config.GetColumnHidingRules().ToImmutableArray());
241247
}
242248

@@ -306,10 +312,8 @@ private static void PrintSummary(ILogger logger, ImmutableConfig config, Summary
306312
logger.WriteLineHeader("// ***** BenchmarkRunner: End *****");
307313
}
308314

309-
private static ImmutableArray<ValidationError> Validate(BenchmarkRunInfo[] benchmarks, ILogger logger)
315+
private static ImmutableArray<ValidationError> Validate(params BenchmarkRunInfo[] benchmarks)
310316
{
311-
logger.WriteLineInfo("// Validating benchmarks:");
312-
313317
var validationErrors = new List<ValidationError>();
314318

315319
if (benchmarks.Any(b => b.Config.Options.IsSet(ConfigOptions.JoinSummary)))
@@ -326,9 +330,6 @@ private static ImmutableArray<ValidationError> Validate(BenchmarkRunInfo[] bench
326330
foreach (var benchmarkRunInfo in benchmarks)
327331
validationErrors.AddRange(benchmarkRunInfo.Config.GetCompositeValidator().Validate(new ValidationParameters(benchmarkRunInfo.BenchmarksCases, benchmarkRunInfo.Config)));
328332

329-
foreach (var validationError in validationErrors.Distinct())
330-
logger.WriteLineError(validationError.Message);
331-
332333
return validationErrors.ToImmutableArray();
333334
}
334335

@@ -531,14 +532,25 @@ private static ExecuteResult RunExecute(ILogger logger, BenchmarkCase benchmarkC
531532
private static void LogTotalTime(ILogger logger, TimeSpan time, int executedBenchmarksCount, string message = "Total time")
532533
=> logger.WriteLineStatistic($"{message}: {time.ToFormattedTotalTime(DefaultCultureInfo.Instance)}, executed benchmarks: {executedBenchmarksCount}");
533534

534-
private static BenchmarkRunInfo[] GetSupportedBenchmarks(BenchmarkRunInfo[] benchmarkRunInfos, ILogger logger, IResolver resolver)
535-
=> benchmarkRunInfos.Select(info => new BenchmarkRunInfo(
536-
info.BenchmarksCases.Where(benchmark => benchmark.GetToolchain().IsSupported(benchmark, logger, resolver)).ToArray(),
535+
private static (BenchmarkRunInfo[], List<ValidationError>) GetSupportedBenchmarks(BenchmarkRunInfo[] benchmarkRunInfos, ILogger logger, IResolver resolver)
536+
{
537+
List<ValidationError> validationErrors = new ();
538+
539+
var benchmarksRunInfo = benchmarkRunInfos.Select(info => new BenchmarkRunInfo(
540+
info.BenchmarksCases.Where(benchmark =>
541+
{
542+
var errors = benchmark.GetToolchain().Validate(benchmark, resolver).ToArray();
543+
validationErrors.AddRange(errors);
544+
return !errors.Any();
545+
}).ToArray(),
537546
info.Type,
538547
info.Config))
539548
.Where(infos => infos.BenchmarksCases.Any())
540549
.ToArray();
541550

551+
return (benchmarkRunInfos, validationErrors);
552+
}
553+
542554
private static string GetRootArtifactsFolderPath(BenchmarkRunInfo[] benchmarkRunInfos)
543555
{
544556
var defaultPath = DefaultConfig.Instance.ArtifactsPath;
@@ -652,5 +664,19 @@ private static TimeSpan GetEstimatedFinishTime(in StartedClock runsChronometer,
652664
TimeSpan fromNow = TimeSpan.FromSeconds(avgSecondsPerBenchmark * benchmarksToRunCount);
653665
return fromNow;
654666
}
667+
668+
private static void PrintValidationErrors(ILogger logger, IEnumerable<ValidationError> validationErrors)
669+
{
670+
foreach (var validationError in validationErrors.Distinct())
671+
{
672+
if (validationError.BenchmarkCase != null)
673+
{
674+
logger.WriteLineInfo($"// Benchmark {validationError.BenchmarkCase.DisplayInfo}");
675+
}
676+
677+
logger.WriteLineError($"// * {validationError.Message}");
678+
logger.WriteLine();
679+
}
680+
}
655681
}
656-
}
682+
}

‎src/BenchmarkDotNet/Running/BenchmarkRunnerDirty.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ private static Summary RunWithExceptionHandling(Func<Summary> run)
136136
catch (InvalidBenchmarkDeclarationException e)
137137
{
138138
ConsoleLogger.Default.WriteLineError(e.Message);
139-
return Summary.NothingToRun(e.Message, string.Empty, string.Empty);
139+
return Summary.ValidationFailed(e.Message, string.Empty, string.Empty);
140140
}
141141
}
142142

@@ -149,7 +149,7 @@ private static Summary[] RunWithExceptionHandling(Func<Summary[]> run)
149149
catch (InvalidBenchmarkDeclarationException e)
150150
{
151151
ConsoleLogger.Default.WriteLineError(e.Message);
152-
return new[] { Summary.NothingToRun(e.Message, string.Empty, string.Empty) };
152+
return new[] { Summary.ValidationFailed(e.Message, string.Empty, string.Empty) };
153153
}
154154
}
155155
}

‎src/BenchmarkDotNet/Running/BuildPartition.cs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
using System;
22
using System.IO;
3-
using System.Linq;
43
using System.Reflection;
54
using BenchmarkDotNet.Characteristics;
65
using BenchmarkDotNet.Configs;
76
using BenchmarkDotNet.Environments;
87
using BenchmarkDotNet.Jobs;
9-
using BenchmarkDotNet.Portability;
10-
using BenchmarkDotNet.Toolchains.NativeAot;
118
using BenchmarkDotNet.Toolchains.CsProj;
129
using BenchmarkDotNet.Toolchains.MonoWasm;
1310
using BenchmarkDotNet.Toolchains.Roslyn;
@@ -51,7 +48,7 @@ public BuildPartition(BenchmarkBuildInfo[] benchmarks, IResolver resolver)
5148
public bool IsNativeAot => RepresentativeBenchmarkCase.Job.IsNativeAOT();
5249

5350
public bool IsWasm => Runtime is WasmRuntime // given job can have Wasm toolchain set, but Runtime == default ;)
54-
|| (RepresentativeBenchmarkCase.Job.Infrastructure.TryGetToolchain(out var toolchain) && toolchain is WasmToolChain);
51+
|| (RepresentativeBenchmarkCase.Job.Infrastructure.TryGetToolchain(out var toolchain) && toolchain is WasmToolchain);
5552

5653
public bool IsNetFramework => Runtime is ClrRuntime
5754
|| (RepresentativeBenchmarkCase.Job.Infrastructure.TryGetToolchain(out var toolchain) && (toolchain is RoslynToolchain || toolchain is CsProjClassicNetToolchain));
@@ -75,4 +72,4 @@ private static string GetResolvedAssemblyLocation(Assembly assembly) =>
7572
// manually construct the path.
7673
assembly.Location.Length == 0 ? Path.Combine(AppContext.BaseDirectory, assembly.GetName().Name) : assembly.Location;
7774
}
78-
}
75+
}

‎src/BenchmarkDotNet/Toolchains/CoreRun/CoreRunToolchain.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.IO;
34
using BenchmarkDotNet.Characteristics;
4-
using BenchmarkDotNet.Loggers;
55
using BenchmarkDotNet.Running;
66
using BenchmarkDotNet.Toolchains.DotNetCli;
7+
using BenchmarkDotNet.Validators;
78

89
namespace BenchmarkDotNet.Toolchains.CoreRun
910
{
@@ -57,18 +58,18 @@ public CoreRunToolchain(FileInfo coreRun, bool createCopy = true,
5758

5859
public override string ToString() => Name;
5960

60-
public bool IsSupported(BenchmarkCase benchmark, ILogger logger, IResolver resolver)
61+
public IEnumerable<ValidationError> Validate(BenchmarkCase benchmark, IResolver resolver)
6162
{
6263
if (!SourceCoreRun.Exists)
6364
{
64-
logger.WriteLineError($"Provided CoreRun path does not exist, benchmark '{benchmark.DisplayInfo}' will not be executed. Please remember that BDN expects path to CoreRun.exe (corerun on Unix), not to Core_Root folder.");
65-
return false;
65+
yield return new ValidationError(true,
66+
$"Provided CoreRun path does not exist, benchmark '{benchmark.DisplayInfo}' will not be executed. Please remember that BDN expects path to CoreRun.exe (corerun on Unix), not to Core_Root folder.",
67+
benchmark);
68+
}
69+
else if (Toolchain.IsCliPathInvalid(CustomDotNetCliPath?.FullName, benchmark, out var invalidCliError))
70+
{
71+
yield return invalidCliError;
6672
}
67-
68-
if (Toolchain.InvalidCliPath(CustomDotNetCliPath?.FullName, benchmark, logger))
69-
return false;
70-
71-
return true;
7273
}
7374

7475
private static FileInfo GetShadowCopyPath(FileInfo coreRunPath)

0 commit comments

Comments
 (0)