From d1b70c15c220352b4dfc48de822622c1a3a0cf0f Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:28 +0300 Subject: [PATCH 01/11] Revert "fixes(configuration): Not unique exporter for exporter type (#1796)" This reverts commit c1f210c7 --- src/BenchmarkDotNet/Configs/DebugConfig.cs | 3 - src/BenchmarkDotNet/Configs/DefaultConfig.cs | 3 - src/BenchmarkDotNet/Configs/IConfig.cs | 5 -- .../Configs/ImmutableConfig.cs | 8 +-- .../Configs/ImmutableConfigBuilder.cs | 70 +++---------------- src/BenchmarkDotNet/Configs/ManualConfig.cs | 4 -- .../Running/BenchmarkConverter.cs | 2 +- .../Running/BenchmarkRunnerClean.cs | 6 -- 8 files changed, 13 insertions(+), 88 deletions(-) diff --git a/src/BenchmarkDotNet/Configs/DebugConfig.cs b/src/BenchmarkDotNet/Configs/DebugConfig.cs index 7c832b9cd3..06e4730dec 100644 --- a/src/BenchmarkDotNet/Configs/DebugConfig.cs +++ b/src/BenchmarkDotNet/Configs/DebugConfig.cs @@ -52,7 +52,6 @@ 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(); @@ -85,7 +84,5 @@ 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 2ee08c8169..4cf97e2956 100644 --- a/src/BenchmarkDotNet/Configs/DefaultConfig.cs +++ b/src/BenchmarkDotNet/Configs/DefaultConfig.cs @@ -20,7 +20,6 @@ namespace BenchmarkDotNet.Configs public class DefaultConfig : IConfig { public static readonly IConfig Instance = new DefaultConfig(); - private readonly static Conclusion[] emptyConclusion = Array.Empty(); private DefaultConfig() { @@ -93,8 +92,6 @@ 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 f3847a6f4e..78f181b9b0 100644 --- a/src/BenchmarkDotNet/Configs/IConfig.cs +++ b/src/BenchmarkDotNet/Configs/IConfig.cs @@ -52,10 +52,5 @@ public interface IConfig /// the auto-generated project build timeout /// TimeSpan BuildTimeout { get; } - - /// - /// Collect any errors or warnings when composing the configuration - /// - IReadOnlyList ConfigAnalysisConclusion { get; } } } diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfig.cs b/src/BenchmarkDotNet/Configs/ImmutableConfig.cs index 83b7b9374a..0f2a926751 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfig.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfig.cs @@ -52,8 +52,7 @@ internal ImmutableConfig( IOrderer orderer, SummaryStyle summaryStyle, ConfigOptions options, - TimeSpan buildTimeout, - IReadOnlyList configAnalysisConclusion) + TimeSpan buildTimeout) { columnProviders = uniqueColumnProviders; loggers = uniqueLoggers; @@ -73,7 +72,6 @@ internal ImmutableConfig( SummaryStyle = summaryStyle; Options = options; BuildTimeout = buildTimeout; - ConfigAnalysisConclusion = configAnalysisConclusion; } public ConfigUnionRule UnionRule { get; } @@ -116,7 +114,5 @@ 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 5aa1746121..0d1bbc0737 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -36,11 +36,10 @@ 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, configAnalyse); + var uniqueExporters = GetExporters(source.GetExporters(), uniqueDiagnosers); var uniqueAnalyzers = GetAnalysers(source.GetAnalysers(), uniqueDiagnosers); var uniqueValidators = GetValidators(source.GetValidators(), MandatoryValidators, source.Options); @@ -69,8 +68,7 @@ public static ImmutableConfig Create(IConfig source) source.Orderer ?? DefaultOrderer.Instance, source.SummaryStyle ?? SummaryStyle.Default, source.Options, - source.BuildTimeout, - configAnalyse.AsReadOnly() + source.BuildTimeout ); } @@ -94,43 +92,18 @@ private static ImmutableHashSet GetDiagnosers(IEnumerable GetExporters(IEnumerable exporters, - ImmutableHashSet uniqueDiagnosers, - IList configAnalyse) + private static ImmutableArray GetExporters(IEnumerable exporters, ImmutableHashSet uniqueDiagnosers) { - - void AddWarning(string message) - { - var conclusion = Conclusion.CreateWarning("Configuration", message); - configAnalyse.Add(conclusion); - } - - var mergeDictionary = new Dictionary(); + var result = new List(); foreach (var exporter in exporters) - { - 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; - } - + if (!result.Contains(exporter)) + result.Add(exporter); foreach (var diagnoser in uniqueDiagnosers) - 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(); - + foreach (var exporter in diagnoser.Exporters) + if (!result.Contains(exporter)) + result.Add(exporter); var hardwareCounterDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); var disassemblyDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); @@ -142,31 +115,8 @@ void AddWarning(string message) for (int i = result.Count - 1; i >=0; i--) if (result[i] is IExporterDependencies exporterDependencies) foreach (var dependency in exporterDependencies.Dependencies) - /* - * 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())) + if (!result.Contains(dependency)) 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 9f632c03d0..6b254b4fc1 100644 --- a/src/BenchmarkDotNet/Configs/ManualConfig.cs +++ b/src/BenchmarkDotNet/Configs/ManualConfig.cs @@ -20,8 +20,6 @@ namespace BenchmarkDotNet.Configs { public class ManualConfig : IConfig { - private readonly static Conclusion[] emptyConclusion = Array.Empty(); - private readonly List columnProviders = new List(); private readonly List exporters = new List(); private readonly List loggers = new List(); @@ -54,8 +52,6 @@ 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); diff --git a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs index 754cf9f9d1..9f6f5a7330 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs @@ -89,7 +89,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 assemblyAttributes.Concat(typeAttributes)) + foreach (var configFromAttribute in typeAttributes.Concat(assemblyAttributes)) 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 c283d40d76..af762d805d 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs @@ -266,12 +266,6 @@ 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.Where(c => c.NeedToShow && !string.IsNullOrEmpty(c.OriginalColumn.Legend)).Select(c => c.OriginalColumn).ToArray(); From 0f76c234ad398e04103987f4866cd221aee57d78 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:29 +0300 Subject: [PATCH 02/11] filter exporters by type+name --- .../Configs/ImmutableConfigBuilder.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 0d1bbc0737..1eef50fab9 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -97,13 +97,11 @@ private static ImmutableArray GetExporters(IEnumerable exp var result = new List(); foreach (var exporter in exporters) - if (!result.Contains(exporter)) - result.Add(exporter); + result.Add(exporter); foreach (var diagnoser in uniqueDiagnosers) foreach (var exporter in diagnoser.Exporters) - if (!result.Contains(exporter)) - result.Add(exporter); + result.Add(exporter); var hardwareCounterDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); var disassemblyDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); @@ -112,15 +110,20 @@ private static ImmutableArray GetExporters(IEnumerable exp if (hardwareCounterDiagnoser != default(IHardwareCountersDiagnoser) && disassemblyDiagnoser != default(DisassemblyDiagnoser)) result.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser)); - for (int i = result.Count - 1; i >=0; i--) + 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)) - result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter + result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter result.Sort((left, right) => (left is IExporterDependencies).CompareTo(right is IExporterDependencies)); // the case when they were defined by user in wrong order ;) - return result.ToImmutableArray(); + var uniqueExporters = new List(); + + foreach (var exporter in result) + if (!uniqueExporters.Contains(exporter, ExporterComparer.Instance)) + uniqueExporters.Add(exporter); + + return uniqueExporters.ToImmutableArray(); } private static ImmutableHashSet GetAnalysers(IEnumerable analysers, ImmutableHashSet uniqueDiagnosers) @@ -198,5 +201,14 @@ private class TypeComparer : IEqualityComparer public int GetHashCode(TInterface obj) => obj.GetType().GetHashCode(); } + + private class ExporterComparer : IEqualityComparer + { + public static readonly ExporterComparer Instance = new ExporterComparer(); + + public bool Equals(IExporter x, IExporter y) => x.GetType() == y.GetType() && x.Name == y.Name; + + public int GetHashCode(IExporter obj) => obj.GetType().GetHashCode(); + } } } From 1d9651f6e6149175b62e73fc16ee28b8e6eac5c8 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:29 +0300 Subject: [PATCH 03/11] redundant? remove --- src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 1eef50fab9..3dcadd48f9 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -115,8 +115,6 @@ private static ImmutableArray GetExporters(IEnumerable exp foreach (var dependency in exporterDependencies.Dependencies) result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter - result.Sort((left, right) => (left is IExporterDependencies).CompareTo(right is IExporterDependencies)); // the case when they were defined by user in wrong order ;) - var uniqueExporters = new List(); foreach (var exporter in result) From 80ae7f37710707bc3e2299e0f80f733208e9f86f Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:29 +0300 Subject: [PATCH 04/11] rename --- .../Configs/ImmutableConfigBuilder.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 3dcadd48f9..30911e0985 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -94,30 +94,30 @@ private static ImmutableHashSet GetDiagnosers(IEnumerable GetExporters(IEnumerable exporters, ImmutableHashSet uniqueDiagnosers) { - var result = new List(); + var allExporters = new List(); foreach (var exporter in exporters) - result.Add(exporter); + allExporters.Add(exporter); foreach (var diagnoser in uniqueDiagnosers) foreach (var exporter in diagnoser.Exporters) - result.Add(exporter); + allExporters.Add(exporter); var hardwareCounterDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); var disassemblyDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); // we can use InstructionPointerExporter only when we have both IHardwareCountersDiagnoser and DisassemblyDiagnoser if (hardwareCounterDiagnoser != default(IHardwareCountersDiagnoser) && disassemblyDiagnoser != default(DisassemblyDiagnoser)) - result.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser)); + allExporters.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser)); - for (int i = result.Count - 1; i >= 0; i--) - if (result[i] is IExporterDependencies exporterDependencies) + for (int i = allExporters.Count - 1; i >= 0; i--) + if (allExporters[i] is IExporterDependencies exporterDependencies) foreach (var dependency in exporterDependencies.Dependencies) - result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter + allExporters.Insert(i, dependency); // All the exporter dependencies should be added before the exporter var uniqueExporters = new List(); - foreach (var exporter in result) + foreach (var exporter in allExporters) if (!uniqueExporters.Contains(exporter, ExporterComparer.Instance)) uniqueExporters.Add(exporter); From e4e1ec18ff233fd53c88fd713cb780cf29187e82 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:29 +0300 Subject: [PATCH 05/11] simplify --- src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 30911e0985..82be00392d 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -107,7 +107,7 @@ private static ImmutableArray GetExporters(IEnumerable exp var disassemblyDiagnoser = uniqueDiagnosers.OfType().SingleOrDefault(); // we can use InstructionPointerExporter only when we have both IHardwareCountersDiagnoser and DisassemblyDiagnoser - if (hardwareCounterDiagnoser != default(IHardwareCountersDiagnoser) && disassemblyDiagnoser != default(DisassemblyDiagnoser)) + if (hardwareCounterDiagnoser != null && disassemblyDiagnoser != null) allExporters.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser)); for (int i = allExporters.Count - 1; i >= 0; i--) From 5f3a1419331091df233e32aa1f0e6da822ee0cbd Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:30 +0300 Subject: [PATCH 06/11] IExporter.Name -> IExporter.Id --- src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs | 2 +- src/BenchmarkDotNet/Exporters/CompositeExporter.cs | 2 +- src/BenchmarkDotNet/Exporters/ExporterBase.cs | 2 +- src/BenchmarkDotNet/Exporters/IExporter.cs | 2 +- src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs | 2 +- src/BenchmarkDotNet/Exporters/RPlotExporter.cs | 2 +- .../BaselineRatioColumnTest.cs | 2 +- tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs | 4 ++-- .../Exporters/CommonExporterApprovalTests.cs | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 82be00392d..557b98ec90 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -204,7 +204,7 @@ private class ExporterComparer : IEqualityComparer { public static readonly ExporterComparer Instance = new ExporterComparer(); - public bool Equals(IExporter x, IExporter y) => x.GetType() == y.GetType() && x.Name == y.Name; + public bool Equals(IExporter x, IExporter y) => x.GetType() == y.GetType() && x.Id == y.Id; public int GetHashCode(IExporter obj) => obj.GetType().GetHashCode(); } diff --git a/src/BenchmarkDotNet/Exporters/CompositeExporter.cs b/src/BenchmarkDotNet/Exporters/CompositeExporter.cs index 95d3bde9d3..e415a9a7aa 100644 --- a/src/BenchmarkDotNet/Exporters/CompositeExporter.cs +++ b/src/BenchmarkDotNet/Exporters/CompositeExporter.cs @@ -14,7 +14,7 @@ public class CompositeExporter : IExporter public CompositeExporter(ImmutableArray exporters) => this.exporters = exporters; - public string Name => nameof(CompositeExporter); + public string Id => nameof(CompositeExporter); public void ExportToLog(Summary summary, ILogger logger) { diff --git a/src/BenchmarkDotNet/Exporters/ExporterBase.cs b/src/BenchmarkDotNet/Exporters/ExporterBase.cs index 17a8d432a6..ca8258abc4 100644 --- a/src/BenchmarkDotNet/Exporters/ExporterBase.cs +++ b/src/BenchmarkDotNet/Exporters/ExporterBase.cs @@ -10,7 +10,7 @@ namespace BenchmarkDotNet.Exporters { public abstract class ExporterBase : IExporter { - public string Name => $"{GetType().Name}{FileNameSuffix}"; + public string Id => $"{GetType().Name}{FileNameSuffix}"; protected virtual string FileExtension => "txt"; protected virtual string FileNameSuffix => string.Empty; diff --git a/src/BenchmarkDotNet/Exporters/IExporter.cs b/src/BenchmarkDotNet/Exporters/IExporter.cs index 19d55b687b..2e5a0bb10b 100644 --- a/src/BenchmarkDotNet/Exporters/IExporter.cs +++ b/src/BenchmarkDotNet/Exporters/IExporter.cs @@ -6,7 +6,7 @@ namespace BenchmarkDotNet.Exporters { public interface IExporter { - string Name { get; } + string Id { get; } void ExportToLog(Summary summary, ILogger logger); IEnumerable ExportToFiles(Summary summary, ILogger consoleLogger); diff --git a/src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs b/src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs index 9bd783b88f..720128cfa4 100644 --- a/src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs +++ b/src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs @@ -31,7 +31,7 @@ internal InstructionPointerExporter(IHardwareCountersDiagnoser hardwareCountersD this.disassemblyDiagnoser = disassemblyDiagnoser; } - public string Name => nameof(InstructionPointerExporter); + public string Id => nameof(InstructionPointerExporter); public void ExportToLog(Summary summary, ILogger logger) { } diff --git a/src/BenchmarkDotNet/Exporters/RPlotExporter.cs b/src/BenchmarkDotNet/Exporters/RPlotExporter.cs index 602f299653..90d1c83386 100644 --- a/src/BenchmarkDotNet/Exporters/RPlotExporter.cs +++ b/src/BenchmarkDotNet/Exporters/RPlotExporter.cs @@ -14,7 +14,7 @@ namespace BenchmarkDotNet.Exporters public class RPlotExporter : IExporter, IExporterDependencies { public static readonly IExporter Default = new RPlotExporter(); - public string Name => nameof(RPlotExporter); + public string Id => nameof(RPlotExporter); private const string ImageExtension = ".png"; private static readonly object BuildScriptLock = new object(); diff --git a/tests/BenchmarkDotNet.IntegrationTests/BaselineRatioColumnTest.cs b/tests/BenchmarkDotNet.IntegrationTests/BaselineRatioColumnTest.cs index 3da377ae6c..feb3529cee 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/BaselineRatioColumnTest.cs +++ b/tests/BenchmarkDotNet.IntegrationTests/BaselineRatioColumnTest.cs @@ -90,7 +90,7 @@ public class TestExporter : IExporter public string Description => "For Testing Only!"; - public string Name => "TestBenchmarkExporter"; + public string Id => "TestBenchmarkExporter"; public void ExportToLog(Summary summary, BenchmarkDotNet.Loggers.ILogger logger) => ExportCalled = true; diff --git a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs index 88bff1070e..5f389fc2df 100644 --- a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs +++ b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs @@ -363,7 +363,7 @@ public IEnumerable Dependencies public IEnumerable ExportToFiles(Summary summary, ILogger consoleLogger) => Enumerable.Empty(); - public string Name => nameof(TestExporter); + public string Id => nameof(TestExporter); public void ExportToLog(Summary summary, ILogger logger) { } } @@ -373,7 +373,7 @@ public class TestExporterDependency : IExporter public IEnumerable ExportToFiles(Summary summary, ILogger consoleLogger) => Enumerable.Empty(); - public string Name => nameof(TestExporterDependency); + public string Id => nameof(TestExporterDependency); public void ExportToLog(Summary summary, ILogger logger) { } } diff --git a/tests/BenchmarkDotNet.Tests/Exporters/CommonExporterApprovalTests.cs b/tests/BenchmarkDotNet.Tests/Exporters/CommonExporterApprovalTests.cs index 0b784bb77a..779cac8f58 100644 --- a/tests/BenchmarkDotNet.Tests/Exporters/CommonExporterApprovalTests.cs +++ b/tests/BenchmarkDotNet.Tests/Exporters/CommonExporterApprovalTests.cs @@ -66,7 +66,7 @@ public void Exporters(string cultureInfoName) private static void PrintTitle(AccumulationLogger logger, IExporter exporter) { logger.WriteLine("############################################"); - logger.WriteLine($"{exporter.Name}"); + logger.WriteLine($"{exporter.Id}"); logger.WriteLine("############################################"); } From 9e69c0350293776c65c73c1e27f218c5aabc5d90 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:30 +0300 Subject: [PATCH 07/11] reorder --- src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs | 4 ++-- src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs b/src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs index 9a1d60cbd8..a0c39e16a1 100644 --- a/src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs +++ b/src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs @@ -6,10 +6,10 @@ namespace BenchmarkDotNet.Exporters { public class AsciiDocExporter : ExporterBase { - protected override string FileExtension => "asciidoc"; - public static readonly IExporter Default = new AsciiDocExporter(); + protected override string FileExtension => "asciidoc"; + private AsciiDocExporter() { } diff --git a/src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs b/src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs index e6ab71ab58..c2c7da4420 100644 --- a/src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs +++ b/src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs @@ -6,12 +6,12 @@ namespace BenchmarkDotNet.Exporters.Csv { public class CsvExporter : ExporterBase { + public static readonly IExporter Default = new CsvExporter(CsvSeparator.CurrentCulture, SummaryStyle.Default.WithZeroMetricValuesInContent()); + private readonly SummaryStyle style; private readonly CsvSeparator separator; protected override string FileExtension => "csv"; - public static readonly IExporter Default = new CsvExporter(CsvSeparator.CurrentCulture, SummaryStyle.Default.WithZeroMetricValuesInContent()); - public CsvExporter(CsvSeparator separator) : this (separator, SummaryStyle.Default.WithZeroMetricValuesInContent()) { } From b3566471056a17a483e2c35f50e5c3d0355d74b0 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:30 +0300 Subject: [PATCH 08/11] Add RPlotValidator --- src/BenchmarkDotNet/Configs/DefaultConfig.cs | 1 + .../Configs/ImmutableConfigBuilder.cs | 3 +- .../Validators/RPlotExporterValidator.cs | 38 +++++++++++++++++++ .../Configs/ImmutableConfigTests.cs | 35 +++++------------ .../Validators/RPlotValidatorTests.cs | 36 ++++++++++++++++++ 5 files changed, 86 insertions(+), 27 deletions(-) create mode 100644 src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs create mode 100644 tests/BenchmarkDotNet.Tests/Validators/RPlotValidatorTests.cs diff --git a/src/BenchmarkDotNet/Configs/DefaultConfig.cs b/src/BenchmarkDotNet/Configs/DefaultConfig.cs index 4cf97e2956..c69975f838 100644 --- a/src/BenchmarkDotNet/Configs/DefaultConfig.cs +++ b/src/BenchmarkDotNet/Configs/DefaultConfig.cs @@ -67,6 +67,7 @@ public IEnumerable GetValidators() yield return GenericBenchmarksValidator.DontFailOnError; yield return DeferredExecutionValidator.FailOnError; yield return ParamsAllValuesValidator.FailOnError; + yield return RPlotExporterValidator.FailOnError; } public IOrderer Orderer => null; diff --git a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs index 557b98ec90..475310f886 100644 --- a/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs +++ b/src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs @@ -26,7 +26,8 @@ public static class ImmutableConfigBuilder ConfigValidator.DontFailOnError, ShadowCopyValidator.DontFailOnError, JitOptimizationsValidator.DontFailOnError, - DeferredExecutionValidator.DontFailOnError + DeferredExecutionValidator.DontFailOnError, + RPlotExporterValidator.FailOnError }; /// diff --git a/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs b/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs new file mode 100644 index 0000000000..d5e5c5cab3 --- /dev/null +++ b/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs @@ -0,0 +1,38 @@ +using System.Collections.Generic; +using System.Linq; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Exporters.Csv; + +namespace BenchmarkDotNet.Validators +{ + public class RPlotExporterValidator : IValidator + { + public static readonly RPlotExporterValidator FailOnError = new RPlotExporterValidator(); + + public bool TreatsWarningsAsErrors => true; + + public IEnumerable Validate(ValidationParameters validationParameters) + { + var exporters = validationParameters.Config.GetExporters(); + if (!ContainRPlotExporter(exporters)) + yield break; + + if (!ValidateCsvExporter(exporters)) + yield return new ValidationError(TreatsWarningsAsErrors, "RPlotExporter requires CsvMeasurementsExporter.Default. Do not override CsvMeasurementsExporter"); + } + + private static bool ContainRPlotExporter(IEnumerable exporters) + { + return exporters.Any(exporter => exporter.GetType() == typeof(RPlotExporter)); + } + + private static bool ValidateCsvExporter(IEnumerable exporters) + { + var exporter = exporters.Cast().FirstOrDefault(); + if (exporter == null) + return false; + + return exporter.Separator == CsvMeasurementsExporter.Default.Separator; + } + } +} \ No newline at end of file diff --git a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs index 5f389fc2df..d997dd42a9 100644 --- a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs +++ b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs @@ -7,6 +7,7 @@ using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Environments; using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Exporters.Csv; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Order; @@ -378,35 +379,17 @@ public void ExportToLog(Summary summary, ILogger logger) { } } [Fact] - public void GenerateWarningWhenExporterDependencyAlreadyExistInConfig() + public void TheFirstCsvExporterHasPrecedence() { - 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; + var config = ManualConfig.CreateEmpty(); + config.AddExporter(CsvMeasurementsExporter.Default); + config.AddExporter(new CsvMeasurementsExporter(CsvSeparator.Comma)); - } + var immutableConfig = config.CreateImmutableConfig(); + var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + Assert.Single(csvExporters); + Assert.True(csvExporters.First() == CsvMeasurementsExporter.Default); } } } diff --git a/tests/BenchmarkDotNet.Tests/Validators/RPlotValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/RPlotValidatorTests.cs new file mode 100644 index 0000000000..594ab1039e --- /dev/null +++ b/tests/BenchmarkDotNet.Tests/Validators/RPlotValidatorTests.cs @@ -0,0 +1,36 @@ +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Exporters.Csv; +using BenchmarkDotNet.Extensions; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Validators; +using Xunit; + +namespace BenchmarkDotNet.Tests.Validators +{ + public class RPlotValidatorTests + { + [Fact] + public void RPlotExporterRequiresDefaultCsvExporter() + { + var actualSeparator = Thread.CurrentThread.CurrentCulture.GetActualListSeparator(); + + var alternativeSeparator = actualSeparator == CsvSeparator.Comma.ToRealSeparator() + ? CsvSeparator.Semicolon + : CsvSeparator.Comma; + + var config = ManualConfig.CreateEmpty(); + config.AddExporter(new CsvMeasurementsExporter(alternativeSeparator)); + config.AddExporter(RPlotExporter.Default); + + var validationParameters = new ValidationParameters(ImmutableArray.Empty, config.CreateImmutableConfig()); + var validationErrors = RPlotExporterValidator.FailOnError.Validate(validationParameters).ToArray(); + + Assert.Single(validationErrors); + Assert.Equal("RPlotExporter requires CsvMeasurementsExporter.Default. Do not override CsvMeasurementsExporter", validationErrors.First().Message); + } + } +} \ No newline at end of file From e46116bd3fc15c10b3e4db29784d7824791807be Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:20:31 +0300 Subject: [PATCH 09/11] add tests --- .../Configs/ImmutableConfigTests.cs | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs index d997dd42a9..1769f65424 100644 --- a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs +++ b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs @@ -8,6 +8,7 @@ using BenchmarkDotNet.Environments; using BenchmarkDotNet.Exporters; using BenchmarkDotNet.Exporters.Csv; +using BenchmarkDotNet.Exporters.Xml; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Order; @@ -350,7 +351,7 @@ private static ImmutableConfig[] AddLeftToTheRightAndRightToTheLef(ManualConfig var leftAddedToTheRight = ManualConfig.Create(right); leftAddedToTheRight.Add(left); - return new[]{ rightAddedToLeft.CreateImmutableConfig(), leftAddedToTheRight.CreateImmutableConfig() }; + return new[] { rightAddedToLeft.CreateImmutableConfig(), leftAddedToTheRight.CreateImmutableConfig() }; } public class TestExporter : IExporter, IExporterDependencies @@ -391,5 +392,37 @@ public void TheFirstCsvExporterHasPrecedence() Assert.Single(csvExporters); Assert.True(csvExporters.First() == CsvMeasurementsExporter.Default); } + + [Fact] + public void DifferentMarkdownExportersAreNotRemoved() + { + var config = ManualConfig.CreateEmpty(); + config.AddExporter(MarkdownExporter.Default); + config.AddExporter(MarkdownExporter.GitHub); + + var immutableConfig = config.CreateImmutableConfig(); + var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + + Assert.Equal(2, csvExporters.Length); + Assert.True(csvExporters[0] == MarkdownExporter.Default); + Assert.True(csvExporters[1] == MarkdownExporter.GitHub); + } + + [Fact] + public void DifferentXmlExportersAreNotRemoved() + { + var config = ManualConfig.CreateEmpty(); + config.AddExporter(XmlExporter.Default); + config.AddExporter(XmlExporter.Full); + config.AddExporter(XmlExporter.FullCompressed); + + var immutableConfig = config.CreateImmutableConfig(); + var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + + Assert.Equal(3, csvExporters.Length); + Assert.True(csvExporters[0] == XmlExporter.Default); + Assert.True(csvExporters[1] == XmlExporter.Full); + Assert.True(csvExporters[2] == XmlExporter.FullCompressed); + } } -} +} \ No newline at end of file From b5eec8495f30a015ef5d7e1080bf9879306c7484 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 18:55:08 +0300 Subject: [PATCH 10/11] rename --- .../Configs/ImmutableConfigTests.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs index 1769f65424..9e282a4cc1 100644 --- a/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs +++ b/tests/BenchmarkDotNet.Tests/Configs/ImmutableConfigTests.cs @@ -387,10 +387,10 @@ public void TheFirstCsvExporterHasPrecedence() config.AddExporter(new CsvMeasurementsExporter(CsvSeparator.Comma)); var immutableConfig = config.CreateImmutableConfig(); - var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + var exporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); - Assert.Single(csvExporters); - Assert.True(csvExporters.First() == CsvMeasurementsExporter.Default); + Assert.Single(exporters); + Assert.True(exporters.First() == CsvMeasurementsExporter.Default); } [Fact] @@ -401,11 +401,11 @@ public void DifferentMarkdownExportersAreNotRemoved() config.AddExporter(MarkdownExporter.GitHub); var immutableConfig = config.CreateImmutableConfig(); - var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + var exporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); - Assert.Equal(2, csvExporters.Length); - Assert.True(csvExporters[0] == MarkdownExporter.Default); - Assert.True(csvExporters[1] == MarkdownExporter.GitHub); + Assert.Equal(2, exporters.Length); + Assert.True(exporters[0] == MarkdownExporter.Default); + Assert.True(exporters[1] == MarkdownExporter.GitHub); } [Fact] @@ -417,12 +417,12 @@ public void DifferentXmlExportersAreNotRemoved() config.AddExporter(XmlExporter.FullCompressed); var immutableConfig = config.CreateImmutableConfig(); - var csvExporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); + var exporters = immutableConfig.GetExporters().Cast().Where(e => e != null).ToArray(); - Assert.Equal(3, csvExporters.Length); - Assert.True(csvExporters[0] == XmlExporter.Default); - Assert.True(csvExporters[1] == XmlExporter.Full); - Assert.True(csvExporters[2] == XmlExporter.FullCompressed); + Assert.Equal(3, exporters.Length); + Assert.True(exporters[0] == XmlExporter.Default); + Assert.True(exporters[1] == XmlExporter.Full); + Assert.True(exporters[2] == XmlExporter.FullCompressed); } } } \ No newline at end of file From b79bc45b64131dd7147d0b68655b84657f6a0006 Mon Sep 17 00:00:00 2001 From: Yegor Stepanov Date: Mon, 17 Oct 2022 20:02:51 +0300 Subject: [PATCH 11/11] Cast -> OfType --- src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs b/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs index d5e5c5cab3..0156cd2a21 100644 --- a/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs +++ b/src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs @@ -28,7 +28,7 @@ private static bool ContainRPlotExporter(IEnumerable exporters) private static bool ValidateCsvExporter(IEnumerable exporters) { - var exporter = exporters.Cast().FirstOrDefault(); + var exporter = exporters.OfType().FirstOrDefault(); if (exporter == null) return false;