Skip to content
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

add quiet logger #2161

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/BenchmarkDotNet/Attributes/QuietModeAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using BenchmarkDotNet.Configs;
using JetBrains.Annotations;
using System;

namespace BenchmarkDotNet.Attributes
{
/// <summary>
/// Run benchmars in quiet mode.
/// </summary>
[PublicAPI]
[AttributeUsage(AttributeTargets.Class)]
public class QuietModeAttribute : Attribute, IConfigSource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO there should be 1 attribute for all verbosity options and 1 cmd option instead of a separate attribute/option for each.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand this, the attribute/options are just different ways of configuring the same mode, with quiet=false you get the same output as before with quiet=true you only get the more relevant output

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm mean about it: #190 (comment)
The PR should ideally implement all options, because it's not easy to add logging levels to the BDN.

It would be intuitive to see the following api:

[ConsoleVerbosity(Verbosity.Diagnostic, writeOutputFromGeneratedProject: true/false)]
[ConsoleVerbosity(Verbosity.Detailed)] // your PR does this level
[ConsoleVerbosity(Verbosity.Normal)]
[ConsoleVerbosity(Verbosity.Minimal)]
[ConsoleVerbosity(Verbosity.Quiet)]

cmd

dotnet run -c Release -- --verbosity diagnostic
dotnet run -c Release -- --verbosity detailed
dotnet run -c Release -- --verbosity normal
dotnet run -c Release -- --verbosity minimal
dotnet run -c Release -- --verbosity quiet
//short form:
dotnet run -c Release -- -v diag
dotnet run -c Release -- -v d
dotnet run -c Release -- -v n
dotnet run -c Release -- -v m
dotnet run -c Release -- -v q

If we implement your PR, it would be hard to do the standard .NET --verbosity option.

And the naming [QuietMode] is super confusing.
Below I have written 3 possible options. By my opinion it should mean 1) or 3).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now the full proposal, I had not seen last Adam's message on the issue, I will work on this during the next couple of days!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this can be quite complicated.

My proposal is not final.
It's worth to post verbosity output to #190 (as mine) to start a discussion (because it affects the BDN a lot)

PS. If you don't do it, I'll get to it in a month. Any progress that you make will be very helpful!!

{
public IConfig Config { get; }

public QuietModeAttribute(bool value = false)
{
Config = ManualConfig.CreateEmpty().WithOption(ConfigOptions.QuietMode, value);
}
}
}
5 changes: 5 additions & 0 deletions src/BenchmarkDotNet/Configs/ConfigExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ public static class ConfigExtensions
[PublicAPI] public static ManualConfig WithUnionRule(this IConfig config, ConfigUnionRule unionRule) => config.With(m => m.WithUnionRule(unionRule));
[PublicAPI] public static ManualConfig WithCultureInfo(this IConfig config, CultureInfo cultureInfo) => config.With(m => m.CultureInfo = cultureInfo);

/// <summary>
/// Run benchmars in quiet mode.
/// </summary>
[PublicAPI] public static IConfig QuietMode(this IConfig config, bool value = true) => config.WithOption(ConfigOptions.QuietMode, value);

/// <summary>
/// determines if all auto-generated files should be kept or removed after running the benchmarks
/// </summary>
Expand Down
6 changes: 5 additions & 1 deletion src/BenchmarkDotNet/Configs/ConfigOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ public enum ConfigOptions
/// <summary>
/// Performs apples-to-apples comparison for provided benchmarks and jobs. Experimental, will change in the near future!
/// </summary>
ApplesToApples = 1 << 8
ApplesToApples = 1 << 8,
/// <summary>
/// Run benchmars in quiet mode.
/// </summary>
QuietMode = 1 << 9
}

internal static class ConfigOptionsExtensions
Expand Down
3 changes: 3 additions & 0 deletions src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ public bool UseDisassemblyDiagnoser
[Option("apples", Required = false, Default = false, HelpText = "Runs apples-to-apples comparison for specified Jobs.")]
public bool ApplesToApples { get; set; }

[Option("quiet", Required = false, Default = false, HelpText = "Run benchmars in quiet mode.")]
public bool QuietMode { get; set; }

[Option("list", Required = false, Default = ListBenchmarkCaseMode.Disabled, HelpText = "Prints all of the available benchmark names. Flat/Tree")]
public ListBenchmarkCaseMode ListBenchmarkCaseMode { get; set; }

Expand Down
1 change: 1 addition & 0 deletions src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ private static IConfig CreateConfig(CommandLineOptions options, IConfig globalCo
config.WithOption(ConfigOptions.LogBuildOutput, options.LogBuildOutput);
config.WithOption(ConfigOptions.GenerateMSBuildBinLog, options.GenerateMSBuildBinLog);
config.WithOption(ConfigOptions.ApplesToApples, options.ApplesToApples);
config.WithOption(ConfigOptions.QuietMode, options.QuietMode);

if (options.MaxParameterColumnWidth.HasValue)
config.WithSummaryStyle(SummaryStyle.Default.WithMaxParameterColumnWidth(options.MaxParameterColumnWidth.Value));
Expand Down
28 changes: 18 additions & 10 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
var cultureInfo = config.CultureInfo ?? DefaultCultureInfo.Instance;
var reports = new List<BenchmarkReport>();
string title = GetTitle(new[] { benchmarkRunInfo });
var quietLogger = GetQuietLogger(config, logger);
var consoleTitle = RuntimeInformation.IsWindows() ? Console.Title : string.Empty;

logger.WriteLineInfo($"// Found {benchmarks.Length} benchmarks:");
Expand All @@ -160,8 +161,9 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,

UpdateTitle(totalBenchmarkCount, benchmarksToRunCount);

using (var powerManagementApplier = new PowerManagementApplier(logger))
using (var powerManagementApplier = new PowerManagementApplier(quietLogger))
{

bool stop = false;

for (int i = 0; i < benchmarks.Length && !stop; i++)
Expand All @@ -187,7 +189,7 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
{
var statistics = report.GetResultRuns().GetStatistics();
var formatter = statistics.CreateNanosecondFormatter(cultureInfo);
logger.WriteLineStatistic(statistics.ToString(cultureInfo, formatter));
quietLogger.WriteLineStatistic(statistics.ToString(cultureInfo, formatter));
}

if (!report.Success && config.Options.IsSet(ConfigOptions.StopOnFirstError))
Expand Down Expand Up @@ -223,7 +225,6 @@ private static Summary Run(BenchmarkRunInfo benchmarkRunInfo,
logger.WriteLine();

benchmarksToRunCount -= stop ? benchmarks.Length - i : 1;

LogProgress(logger, in runsChronometer, totalBenchmarkCount, benchmarksToRunCount);
}
}
Expand Down Expand Up @@ -391,9 +392,10 @@ private static BuildResult Build(BuildPartition buildPartition, string rootArtif
private static BenchmarkReport RunCore(BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, ILogger logger, IResolver resolver, BuildResult buildResult)
{
var toolchain = benchmarkCase.GetToolchain();
ILogger quietLogger = !benchmarkCase.Config.Options.IsSet(ConfigOptions.QuietMode) ? logger : NullLogger.Instance;

logger.WriteLineHeader("// **************************");
logger.WriteLineHeader("// Benchmark: " + benchmarkCase.DisplayInfo);
quietLogger.WriteLineHeader("// **************************");
quietLogger.WriteLineHeader("// Benchmark: " + benchmarkCase.DisplayInfo);

var (success, executeResults, metrics) = Execute(logger, benchmarkCase, benchmarkId, toolchain, buildResult, resolver);

Expand All @@ -405,8 +407,9 @@ private static (bool success, List<ExecuteResult> executeResults, List<Metric> m
{
var executeResults = new List<ExecuteResult>();
var metrics = new List<Metric>();
var quietLogger = GetQuietLogger(benchmarkCase.Config, logger);

logger.WriteLineInfo("// *** Execute ***");
quietLogger.WriteLineInfo("// *** Execute ***");
bool analyzeRunToRunVariance = benchmarkCase.Job.ResolveValue(AccuracyMode.AnalyzeLaunchVarianceCharacteristic, resolver);
bool autoLaunchCount = !benchmarkCase.Job.HasValue(RunMode.LaunchCountCharacteristic);
int defaultValue = analyzeRunToRunVariance ? 2 : 1;
Expand All @@ -421,7 +424,7 @@ private static (bool success, List<ExecuteResult> executeResults, List<Metric> m
string printedLaunchCount = analyzeRunToRunVariance && autoLaunchCount && launchIndex <= 2
? ""
: " / " + launchCount;
logger.WriteLineInfo($"// Launch: {launchIndex}{printedLaunchCount}");
quietLogger.WriteLineInfo($"// Launch: {launchIndex}{printedLaunchCount}");

// use diagnoser only for the last run (we need single result, not many)
bool useDiagnoser = launchIndex == launchCount && noOverheadCompositeDiagnoser != null;
Expand Down Expand Up @@ -499,12 +502,13 @@ private static (bool success, List<ExecuteResult> executeResults, List<Metric> m
private static ExecuteResult RunExecute(ILogger logger, BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, IToolchain toolchain,
BuildResult buildResult, IResolver resolver, IDiagnoser diagnoser, int launchIndex)
{
var quietLogger = GetQuietLogger(benchmarkCase.Config, logger);
var executeResult = toolchain.Executor.Execute(
new ExecuteParameters(
buildResult,
benchmarkCase,
benchmarkId,
logger,
quietLogger,
resolver,
launchIndex,
diagnoser));
Expand All @@ -518,11 +522,11 @@ private static ExecuteResult RunExecute(ILogger logger, BenchmarkCase benchmarkC
{
if (executeResult.ExitCode is int exitCode)
{
logger.WriteLineInfo($"// Benchmark Process {executeResult.ProcessId} has exited with code {exitCode}.");
quietLogger.WriteLineInfo($"// Benchmark Process {executeResult.ProcessId} has exited with code {exitCode}.");
}
else
{
logger.WriteLineInfo($"// Benchmark Process {executeResult.ProcessId} failed to exit.");
quietLogger.WriteLineInfo($"// Benchmark Process {executeResult.ProcessId} failed to exit.");
}
}

Expand Down Expand Up @@ -678,5 +682,9 @@ private static void PrintValidationErrors(ILogger logger, IEnumerable<Validation
logger.WriteLine();
}
}

private static ILogger GetQuietLogger(ImmutableConfig config, ILogger logger)
=> !config.Options.IsSet(ConfigOptions.QuietMode) ? logger : NullLogger.Instance;

}
}