diff --git a/src/BenchmarkDotNet/Analysers/ConclusionHelper.cs b/src/BenchmarkDotNet/Analysers/ConclusionHelper.cs index 32ab2efa80..9542230606 100644 --- a/src/BenchmarkDotNet/Analysers/ConclusionHelper.cs +++ b/src/BenchmarkDotNet/Analysers/ConclusionHelper.cs @@ -7,14 +7,14 @@ namespace BenchmarkDotNet.Analysers { public static class ConclusionHelper { - public static void Print(ILogger logger, List conclusions) + public static void Print(ILogger logger, IEnumerable 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 conclusions, ConclusionKind kind, string title, Action printLine) + private static void PrintFiltered(IEnumerable conclusions, ConclusionKind kind, string title, Action printLine) { var filtered = conclusions.Where(c => c.Kind == kind).ToArray(); if (filtered.Any()) diff --git a/src/BenchmarkDotNet/Configs/DebugConfig.cs b/src/BenchmarkDotNet/Configs/DebugConfig.cs index 453ee19241..8387cf95e1 100644 --- a/src/BenchmarkDotNet/Configs/DebugConfig.cs +++ b/src/BenchmarkDotNet/Configs/DebugConfig.cs @@ -52,6 +52,7 @@ public override IEnumerable GetJobs() public abstract class DebugConfig : IConfig { + private readonly static Conclusion[] emptyConclusion = Array.Empty(); public abstract IEnumerable GetJobs(); public IEnumerable GetValidators() => Array.Empty(); @@ -83,5 +84,7 @@ public string ArtifactsPath public IEnumerable GetLogicalGroupRules() => Array.Empty(); public ConfigOptions Options => ConfigOptions.KeepBenchmarkFiles | ConfigOptions.DisableOptimizationsValidator; + + public IReadOnlyList ConfigAnalysisConclusion => emptyConclusion; } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Configs/DefaultConfig.cs b/src/BenchmarkDotNet/Configs/DefaultConfig.cs index 315b94ed84..8b893c1dc5 100644 --- a/src/BenchmarkDotNet/Configs/DefaultConfig.cs +++ b/src/BenchmarkDotNet/Configs/DefaultConfig.cs @@ -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(); private DefaultConfig() { @@ -91,6 +92,8 @@ public string ArtifactsPath } } + public IReadOnlyList ConfigAnalysisConclusion => emptyConclusion; + public IEnumerable GetJobs() => Array.Empty(); public IEnumerable GetLogicalGroupRules() => Array.Empty(); diff --git a/src/BenchmarkDotNet/Configs/IConfig.cs b/src/BenchmarkDotNet/Configs/IConfig.cs index 4acb473192..d1cd6dfec6 100644 --- a/src/BenchmarkDotNet/Configs/IConfig.cs +++ b/src/BenchmarkDotNet/Configs/IConfig.cs @@ -51,5 +51,10 @@ public interface IConfig /// the auto-generated project build timeout /// TimeSpan BuildTimeout { get; } + + /// + /// Collect any errors or warnings when composing the configuration + /// + IReadOnlyList ConfigAnalysisConclusion { get; } } -} \ No newline at end of file +} diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfig.cs b/src/BenchmarkDotNet/Configs/ImmutableConfig.cs index 75eaf32b85..429541cc61 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfig.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfig.cs @@ -50,7 +50,8 @@ internal ImmutableConfig( IOrderer orderer, SummaryStyle summaryStyle, ConfigOptions options, - TimeSpan buildTimeout) + TimeSpan buildTimeout, + IReadOnlyList configAnalysisConclusion) { columnProviders = uniqueColumnProviders; loggers = uniqueLoggers; @@ -69,6 +70,7 @@ internal ImmutableConfig( SummaryStyle = summaryStyle; Options = options; BuildTimeout = buildTimeout; + ConfigAnalysisConclusion = configAnalysisConclusion; } public ConfigUnionRule UnionRule { get; } @@ -108,5 +110,7 @@ public IDiagnoser GetCompositeDiagnoser(BenchmarkCase benchmarkCase, RunMode run return diagnosersForGivenMode.Any() ? new CompositeDiagnoser(diagnosersForGivenMode) : null; } + + public IReadOnlyList ConfigAnalysisConclusion { get; private set; } } -} \ No newline at end of file +} diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 8bfdb7c90d..f70f123af5 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -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(); 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); @@ -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() ); } @@ -90,18 +92,43 @@ private static ImmutableHashSet GetDiagnosers(IEnumerable GetExporters(IEnumerable exporters, ImmutableHashSet uniqueDiagnosers) + private static ImmutableArray GetExporters(IEnumerable exporters, + ImmutableHashSet uniqueDiagnosers, + IList configAnalyse) { - var result = new List(); + + void AddWarning(string message) + { + var conclusion = Conclusion.CreateWarning("Configuration", message); + configAnalyse.Add(conclusion); + } + + var mergeDictionary = new Dictionary(); 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().SingleOrDefault(); var disassemblyDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); @@ -113,8 +140,31 @@ private static ImmutableArray GetExporters(IEnumerable 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 ;) diff --git a/src/BenchmarkDotNet/Configs/ManualConfig.cs b/src/BenchmarkDotNet/Configs/ManualConfig.cs index 48fa80384d..2c25ea3321 100644 --- a/src/BenchmarkDotNet/Configs/ManualConfig.cs +++ b/src/BenchmarkDotNet/Configs/ManualConfig.cs @@ -29,7 +29,8 @@ public class ManualConfig : IConfig private readonly List jobs = new List(); private readonly HashSet hardwareCounters = new HashSet(); private readonly List filters = new List(); - private readonly List logicalGroupRules = new List(); + private readonly HashSet logicalGroupRules = new HashSet(); + private readonly static Conclusion[] emptyConclusion = Array.Empty(); public IEnumerable GetColumnProviders() => columnProviders; public IEnumerable GetExporters() => exporters; @@ -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 ConfigAnalysisConclusion => emptyConclusion; + public ManualConfig WithOption(ConfigOptions option, bool value) { Options = Options.Set(value, option); @@ -277,4 +280,4 @@ private static TimeSpan GetBuildTimeout(TimeSpan current, TimeSpan other) ? other : TimeSpan.FromMilliseconds(Math.Max(current.TotalMilliseconds, other.TotalMilliseconds)); } -} \ No newline at end of file +} diff --git a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs index 86b5e67105..981ae41235 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs @@ -88,7 +88,7 @@ private static ImmutableConfig GetFullTypeConfig(Type type, IConfig config) var typeAttributes = type.GetCustomAttributes(true).OfType(); var assemblyAttributes = type.Assembly.GetCustomAttributes().OfType(); - foreach (var configFromAttribute in typeAttributes.Concat(assemblyAttributes)) + foreach (var configFromAttribute in assemblyAttributes.Concat(typeAttributes)) config = ManualConfig.Union(config, configFromAttribute.Config); return ImmutableConfigBuilder.Create(config); diff --git a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs index 784301d315..0bd5a335a8 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs @@ -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; diff --git a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs index 07075c21f5..a7f6f8f8bd 100644 --- a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs +++ b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs @@ -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; + + } + + } } }