Skip to content
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Analysers/ConclusionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ namespace BenchmarkDotNet.Analysers
{
public static class ConclusionHelper
{
public static void Print(ILogger logger, List<Conclusion> conclusions)
public static void Print(ILogger logger, IEnumerable<Conclusion> conclusions)
{
PrintFiltered(conclusions, ConclusionKind.Error, "Errors", logger.WriteLineError);
PrintFiltered(conclusions, ConclusionKind.Warning, "Warnings", logger.WriteLineError);
PrintFiltered(conclusions, ConclusionKind.Hint, "Hints", logger.WriteLineHint);
}

private static void PrintFiltered(List<Conclusion> conclusions, ConclusionKind kind, string title, Action<string> printLine)
private static void PrintFiltered(IEnumerable<Conclusion> conclusions, ConclusionKind kind, string title, Action<string> printLine)
{
var filtered = conclusions.Where(c => c.Kind == kind).ToArray();
if (filtered.Any())
Expand Down
3 changes: 3 additions & 0 deletions src/BenchmarkDotNet/Configs/DebugConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public override IEnumerable<Job> GetJobs()

public abstract class DebugConfig : IConfig
{
private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();
public abstract IEnumerable<Job> GetJobs();

public IEnumerable<IValidator> GetValidators() => Array.Empty<IValidator>();
Expand Down Expand Up @@ -83,5 +84,7 @@ public string ArtifactsPath
public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => Array.Empty<BenchmarkLogicalGroupRule>();

public ConfigOptions Options => ConfigOptions.KeepBenchmarkFiles | ConfigOptions.DisableOptimizationsValidator;

public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;
}
}
3 changes: 3 additions & 0 deletions src/BenchmarkDotNet/Configs/DefaultConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace BenchmarkDotNet.Configs
public class DefaultConfig : IConfig
{
public static readonly IConfig Instance = new DefaultConfig();
private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();

private DefaultConfig()
{
Expand Down Expand Up @@ -91,6 +92,8 @@ public string ArtifactsPath
}
}

public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;

public IEnumerable<Job> GetJobs() => Array.Empty<Job>();

public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => Array.Empty<BenchmarkLogicalGroupRule>();
Expand Down
7 changes: 6 additions & 1 deletion src/BenchmarkDotNet/Configs/IConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,10 @@ public interface IConfig
/// the auto-generated project build timeout
/// </summary>
TimeSpan BuildTimeout { get; }

/// <summary>
/// Collect any errors or warnings when composing the configuration
/// </summary>
IReadOnlyList<Conclusion> ConfigAnalysisConclusion { get; }
}
}
}
8 changes: 6 additions & 2 deletions src/BenchmarkDotNet/Configs/ImmutableConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ internal ImmutableConfig(
IOrderer orderer,
SummaryStyle summaryStyle,
ConfigOptions options,
TimeSpan buildTimeout)
TimeSpan buildTimeout,
IReadOnlyList<Conclusion> configAnalysisConclusion)
{
columnProviders = uniqueColumnProviders;
loggers = uniqueLoggers;
Expand All @@ -69,6 +70,7 @@ internal ImmutableConfig(
SummaryStyle = summaryStyle;
Options = options;
BuildTimeout = buildTimeout;
ConfigAnalysisConclusion = configAnalysisConclusion;
}

public ConfigUnionRule UnionRule { get; }
Expand Down Expand Up @@ -108,5 +110,7 @@ public IDiagnoser GetCompositeDiagnoser(BenchmarkCase benchmarkCase, RunMode run

return diagnosersForGivenMode.Any() ? new CompositeDiagnoser(diagnosersForGivenMode) : null;
}

public IReadOnlyList<Conclusion> ConfigAnalysisConclusion { get; private set; }
}
}
}
70 changes: 60 additions & 10 deletions src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ public static ImmutableConfig Create(IConfig source)
{
var uniqueColumnProviders = source.GetColumnProviders().Distinct().ToImmutableArray();
var uniqueLoggers = source.GetLoggers().ToImmutableHashSet();
var configAnalyse = new List<Conclusion>();

var uniqueHardwareCounters = source.GetHardwareCounters().ToImmutableHashSet();
var uniqueDiagnosers = GetDiagnosers(source.GetDiagnosers(), uniqueHardwareCounters);
var uniqueExporters = GetExporters(source.GetExporters(), uniqueDiagnosers);
var uniqueExporters = GetExporters(source.GetExporters(), uniqueDiagnosers, configAnalyse);
var uniqueAnalyzers = GetAnalysers(source.GetAnalysers(), uniqueDiagnosers);

var uniqueValidators = GetValidators(source.GetValidators(), MandatoryValidators, source.Options);
Expand All @@ -66,7 +67,8 @@ public static ImmutableConfig Create(IConfig source)
source.Orderer ?? DefaultOrderer.Instance,
source.SummaryStyle ?? SummaryStyle.Default,
source.Options,
source.BuildTimeout
source.BuildTimeout,
configAnalyse.AsReadOnly()
);
}

Expand All @@ -90,18 +92,43 @@ private static ImmutableHashSet<IDiagnoser> GetDiagnosers(IEnumerable<IDiagnoser
return builder.ToImmutable();
}

private static ImmutableArray<IExporter> GetExporters(IEnumerable<IExporter> exporters, ImmutableHashSet<IDiagnoser> uniqueDiagnosers)
private static ImmutableArray<IExporter> GetExporters(IEnumerable<IExporter> exporters,
ImmutableHashSet<IDiagnoser> uniqueDiagnosers,
IList<Conclusion> configAnalyse)
{
var result = new List<IExporter>();

void AddWarning(string message)
{
var conclusion = Conclusion.CreateWarning("Configuration", message);
configAnalyse.Add(conclusion);
}

var mergeDictionary = new Dictionary<System.Type, IExporter>();

foreach (var exporter in exporters)
if (!result.Contains(exporter))
result.Add(exporter);
{
var exporterType = exporter.GetType();
if (mergeDictionary.ContainsKey(exporterType))
{
AddWarning($"The exporter {exporterType} is already present in configuration. There may be unexpected results.");
}
mergeDictionary[exporterType] = exporter;
}


foreach (var diagnoser in uniqueDiagnosers)
foreach (var exporter in diagnoser.Exporters)
if (!result.Contains(exporter))
result.Add(exporter);
foreach (var exporter in diagnoser.Exporters)
{
var exporterType = exporter.GetType();
if (mergeDictionary.ContainsKey(exporterType))
{
AddWarning($"The exporter {exporterType} of {diagnoser.GetType().Name} is already present in configuration. There may be unexpected results.");
}
mergeDictionary[exporterType] = exporter;
};

var result = mergeDictionary.Values.ToList();


var hardwareCounterDiagnoser = uniqueDiagnosers.OfType<IHardwareCountersDiagnoser>().SingleOrDefault();
var disassemblyDiagnoser = uniqueDiagnosers.OfType<DisassemblyDiagnoser>().SingleOrDefault();
Expand All @@ -113,8 +140,31 @@ private static ImmutableArray<IExporter> GetExporters(IEnumerable<IExporter> exp
for (int i = result.Count - 1; i >=0; i--)
if (result[i] is IExporterDependencies exporterDependencies)
foreach (var dependency in exporterDependencies.Dependencies)
if (!result.Contains(dependency))
/*
* When exporter that depends on an other already present in the configuration print warning.
*
* Example:
*
* // Global Current Culture separator is Semicolon;
* [CsvMeasurementsExporter(CsvSeparator.Comma)] // force use Comma
* [RPlotExporter]
* public class MyBanch
* {
*
* }
*
* RPlotExporter is depend from CsvMeasurementsExporter.Default
*
* On active logger will by print:
* "The CsvMeasurementsExporter is already present in the configuration. There may be unexpected results of RPlotExporter.
*
*/
if (!result.Any(exporter=> exporter.GetType() == dependency.GetType()))
result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter
else
{
AddWarning($"The {dependency.Name} is already present in the configuration. There may be unexpected results of {result[i].GetType().Name}.");
}

result.Sort((left, right) => (left is IExporterDependencies).CompareTo(right is IExporterDependencies)); // the case when they were defined by user in wrong order ;)

Expand Down
7 changes: 5 additions & 2 deletions src/BenchmarkDotNet/Configs/ManualConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public class ManualConfig : IConfig
private readonly List<Job> jobs = new List<Job>();
private readonly HashSet<HardwareCounter> hardwareCounters = new HashSet<HardwareCounter>();
private readonly List<IFilter> filters = new List<IFilter>();
private readonly List<BenchmarkLogicalGroupRule> logicalGroupRules = new List<BenchmarkLogicalGroupRule>();
private readonly HashSet<BenchmarkLogicalGroupRule> logicalGroupRules = new HashSet<BenchmarkLogicalGroupRule>();
private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();
Comment on lines -32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

@AndreyAkinshin @workgroupengineering
Not sure if you noticed, but this turns logicalGroupRules back into HashSet<> (changed to List<> in #1866).
(I didn't do a full review, but this one jumped at me)

Copy link
Member

Choose a reason for hiding this comment

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

@mawosoft nice catch! Fixed in bd08722


public IEnumerable<IColumnProvider> GetColumnProviders() => columnProviders;
public IEnumerable<IExporter> GetExporters() => exporters;
Expand All @@ -50,6 +51,8 @@ public class ManualConfig : IConfig
[PublicAPI] public SummaryStyle SummaryStyle { get; set; }
[PublicAPI] public TimeSpan BuildTimeout { get; set; } = DefaultConfig.Instance.BuildTimeout;

public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;

public ManualConfig WithOption(ConfigOptions option, bool value)
{
Options = Options.Set(value, option);
Expand Down Expand Up @@ -277,4 +280,4 @@ private static TimeSpan GetBuildTimeout(TimeSpan current, TimeSpan other)
? other
: TimeSpan.FromMilliseconds(Math.Max(current.TotalMilliseconds, other.TotalMilliseconds));
}
}
}
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private static ImmutableConfig GetFullTypeConfig(Type type, IConfig config)
var typeAttributes = type.GetCustomAttributes(true).OfType<IConfigSource>();
var assemblyAttributes = type.Assembly.GetCustomAttributes().OfType<IConfigSource>();

foreach (var configFromAttribute in typeAttributes.Concat(assemblyAttributes))
foreach (var configFromAttribute in assemblyAttributes.Concat(typeAttributes))
config = ManualConfig.Union(config, configFromAttribute.Config);

return ImmutableConfigBuilder.Create(config);
Expand Down
6 changes: 6 additions & 0 deletions src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ private static void PrintSummary(ILogger logger, ImmutableConfig config, Summary
// TODO: make exporter
ConclusionHelper.Print(logger, config.GetCompositeAnalyser().Analyse(summary).Distinct().ToList());

if (config.ConfigAnalysisConclusion.Any())
{
logger.WriteLineHeader("// * Config Issues *");
ConclusionHelper.Print(logger, config.ConfigAnalysisConclusion);
}

// TODO: move to conclusions
var columnWithLegends = summary.Table.Columns.Select(c => c.OriginalColumn).Where(c => !string.IsNullOrEmpty(c.Legend)).ToList();
var effectiveTimeUnit = summary.Table.EffectiveSummaryStyle.TimeUnit;
Expand Down
32 changes: 32 additions & 0 deletions tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -376,5 +376,37 @@ public class TestExporterDependency : IExporter
public string Name => nameof(TestExporterDependency);
public void ExportToLog(Summary summary, ILogger logger) { }
}

[Fact]
public void GenerateWarningWhenExporterDependencyAlreadyExistInConfig()
{
System.Globalization.CultureInfo currentCulture = default;
System.Globalization.CultureInfo currentUICulture = default;
{
var ct = System.Threading.Thread.CurrentThread;
currentCulture = ct.CurrentCulture;
currentUICulture = ct.CurrentUICulture;
ct.CurrentCulture = System.Globalization.CultureInfo.InvariantCulture;
ct.CurrentUICulture = System.Globalization.CultureInfo.InvariantCulture;
}
try
{
var mutable = ManualConfig.CreateEmpty();
mutable.AddExporter(new BenchmarkDotNet.Exporters.Csv.CsvMeasurementsExporter(BenchmarkDotNet.Exporters.Csv.CsvSeparator.Comma));
mutable.AddExporter(RPlotExporter.Default);

var final = ImmutableConfigBuilder.Create(mutable);

Assert.Equal(1, final.ConfigAnalysisConclusion.Count);
}
finally
{
var ct = System.Threading.Thread.CurrentThread;
ct.CurrentCulture = currentCulture;
ct.CurrentUICulture = currentUICulture;

}

}
}
}