Skip to content

Commit

Permalink
Cleanup: Common Practices and Code Improvements in BenchmarkDotNet
Browse files Browse the repository at this point in the history
  • Loading branch information
AndreyAkinshin committed Jul 28, 2018
1 parent 5f84526 commit a76f438
Show file tree
Hide file tree
Showing 103 changed files with 448 additions and 417 deletions.
6 changes: 4 additions & 2 deletions BenchmarkDotNet.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,21 @@
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeAccessorOwnerBody/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeConstructorOrDestructorBody/@EntryIndexedValue"></s:String>
<s:Boolean x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeConstructorOrDestructorBody/@EntryIndexRemoved">True</s:Boolean>

<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeLocalFunctionBody/@EntryIndexedValue"></s:String>
<s:Boolean x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeLocalFunctionBody/@EntryIndexRemoved">True</s:Boolean>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeMethodOrOperatorBody/@EntryIndexedValue"></s:String>
<s:Boolean x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ArrangeMethodOrOperatorBody/@EntryIndexRemoved">True</s:Boolean>

<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ConvertIfStatementToReturnStatement/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=InvertIf/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=RedundantArgumentDefaultValue/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=RedundantAttributeUsageProperty/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=RedundantExplicitTupleComponentName/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=RemoveRedundantBraces/@EntryIndexedValue"></s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=SuggestBaseTypeForParameter/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ParameterTypeCanBeEnumerable_002ELocal/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ParameterTypeCanBeEnumerable_002EGlobal/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ReturnTypeCanBeEnumerable_002EGlobal/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=ReturnTypeCanBeEnumerable_002ELocal/@EntryIndexedValue">DO_NOT_SHOW</s:String>
<s:Boolean x:Key="/Default/CodeInspection/Highlighting/ReadSettingsFromFileLevel/@EntryValue">True</s:Boolean>
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpCodeStyle/BRACES_FOR_IFELSE/@EntryValue">NotRequired</s:String>
<s:String x:Key="/Default/CodeStyle/CodeFormatting/CSharpCodeStyle/CONSTRUCTOR_OR_DESTRUCTOR_BODY/@EntryValue">ExpressionBody</s:String>
Expand Down
10 changes: 5 additions & 5 deletions src/BenchmarkDotNet/Analysers/AnalyserBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ public IEnumerable<Conclusion> Analyse(Summary summary)
yield return conclusion;
}

public virtual IEnumerable<Conclusion> AnalyseSummary(Summary summary) => Enumerable.Empty<Conclusion>();
public virtual IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary) => Enumerable.Empty<Conclusion>();
protected virtual IEnumerable<Conclusion> AnalyseSummary(Summary summary) => Enumerable.Empty<Conclusion>();
protected virtual IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary) => Enumerable.Empty<Conclusion>();

public Conclusion CreateHint(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateHint(Id, message, report);
public Conclusion CreateWarning(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateWarning(Id, message, report);
public Conclusion CreateError(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateError(Id, message, report);
protected Conclusion CreateHint(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateHint(Id, message, report);
protected Conclusion CreateWarning(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateWarning(Id, message, report);
protected Conclusion CreateError(string message, [CanBeNull] BenchmarkReport report = null) => Conclusion.CreateError(Id, message, report);
}
}
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Analysers/EnvironmentAnalyser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ private EnvironmentAnalyser()
{
}

public override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
protected override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
{
if (report.BenchmarkCase.Descriptor.Type.GetTypeInfo().Assembly.IsJitOptimizationDisabled().IsTrue())
yield return CreateWarning("Benchmark was built without optimization enabled (most probably a DEBUG configuration). Please, build it in RELEASE.", report);
}

public override IEnumerable<Conclusion> AnalyseSummary(Summary summary)
protected override IEnumerable<Conclusion> AnalyseSummary(Summary summary)
{
if (summary.HostEnvironmentInfo.HasAttachedDebugger)
yield return CreateWarning("Benchmark was executed with attached debugger");
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Analysers/MinIterationTimeAnalyser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private MinIterationTimeAnalyser()
{
}

public override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
protected override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
{
var target = report.AllMeasurements.Where(m => m.Is(IterationMode.Workload, IterationStage.Actual)).ToArray();
if (target.IsEmpty())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ private MultimodalDistributionAnalyzer() { }

public override string Id => "MultimodalDistribution";

public override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
protected override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
{
var statistics = report.ResultStatistics;
if (statistics == null || statistics.N < EngineResolver.DefaultMinWorkloadIterationCount)
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Analysers/OutliersAnalyser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class OutliersAnalyser : AnalyserBase

private OutliersAnalyser() { }

public override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
protected override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
{
var workloadActual = report.AllMeasurements.Where(m => m.Is(IterationMode.Workload, IterationStage.Actual)).ToArray();
if (workloadActual.IsEmpty())
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Analysers/RuntimeErrorAnalyser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ private RuntimeErrorAnalyser()
{
}

public override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
protected override IEnumerable<Conclusion> AnalyseReport(BenchmarkReport report, Summary summary)
{
var errors = report.ExecuteResults.SelectMany(r => r.Data)
.Union(report.ExecuteResults.SelectMany(r => r.ExtraOutput))
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Attributes/ArgumentsAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace BenchmarkDotNet.Attributes
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
public class ArgumentsAttribute : Attribute
{
public object[] Values { get; private set; }
public object[] Values { get; }

// CLS-Compliant Code requires a constructor without an array in the argument list
public ArgumentsAttribute()
Expand Down
2 changes: 2 additions & 0 deletions src/BenchmarkDotNet/Attributes/ArtifactsPathAttribute.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
using System;
using BenchmarkDotNet.Configs;
using JetBrains.Annotations;

namespace BenchmarkDotNet.Attributes
{
[PublicAPI]
public class ArtifactsPathAttribute : Attribute, IConfigSource
{
public string Value { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace BenchmarkDotNet.Attributes
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Assembly, AllowMultiple = true)]
public class JsonExporterAttribute : ExporterConfigBaseAttribute
{
protected JsonExporterAttribute(IExporter exporter) : base(exporter)
private JsonExporterAttribute(IExporter exporter) : base(exporter)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace BenchmarkDotNet.Attributes
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Assembly, AllowMultiple = true)]
public class XmlExporterAttribute : ExporterConfigBaseAttribute
{
public XmlExporterAttribute(IExporter exporter) : base(exporter)
private XmlExporterAttribute(IExporter exporter) : base(exporter)
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Attributes/Jobs/JobConfigbaseAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ public class JobConfigBaseAttribute : Attribute, IConfigSource
{
// CLS-Compliant Code requires a constructor which use only CLS-compliant types
public JobConfigBaseAttribute() => Config = ManualConfig.CreateEmpty();
public JobConfigBaseAttribute(Job job) => Config = ManualConfig.CreateEmpty().With(job);

protected JobConfigBaseAttribute(Job job) => Config = ManualConfig.CreateEmpty().With(job);

public IConfig Config { get; }
}
Expand Down
2 changes: 0 additions & 2 deletions src/BenchmarkDotNet/Attributes/KeepBenchmarkFilesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ namespace BenchmarkDotNet.Attributes
{
public class KeepBenchmarkFilesAttribute : Attribute, IConfigSource
{
public bool Value { get; }
public IConfig Config { get; }

public KeepBenchmarkFilesAttribute(bool value = true)
{
Value = value;
Config = ManualConfig.CreateEmpty().KeepBenchmarkFiles(value);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Attributes/ParamsAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace BenchmarkDotNet.Attributes
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public class ParamsAttribute : Attribute
{
public object[] Values { get; private set; }
public object[] Values { get; }

// CLS-Compliant Code requires a constructor without an array in the argument list
public ParamsAttribute()
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Characteristics/Characteristic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected Characteristic(

public Type DeclaringType { get; }

public object FallbackValue { get; }
private object FallbackValue { get; }

public object this[CharacteristicObject obj]
{
Expand Down
3 changes: 1 addition & 2 deletions src/BenchmarkDotNet/Characteristics/CharacteristicHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ public static class CharacteristicHelper
internal static bool IsCharacteristicObjectSubclass(Type type) =>
type.GetTypeInfo().IsSubclassOf(typeof(CharacteristicObject));

internal static bool IsCharacteristicSubclass(Type type) =>
private static bool IsCharacteristicSubclass(Type type) =>
type.GetTypeInfo().IsSubclassOf(typeof(Characteristic));

private static Characteristic AssertHasValue(MemberInfo member, Characteristic value)
{
// ReSharper disable once PossibleNullReferenceException
if (value == null)
throw new ArgumentException(
$"The value of {member.DeclaringType.Name}.{member.Name} is null");
Expand Down
12 changes: 7 additions & 5 deletions src/BenchmarkDotNet/Characteristics/CharacteristicObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,16 @@ protected CharacteristicObject(string id) : this()
#endregion

#region Assertions
protected void AssertNotFrozen()

private void AssertNotFrozen()
{
if (Frozen)
{
throw new InvalidOperationException($"The current object {this} is frozen. Create a copy to modify.");
}
}

protected void AssertIsRoot()
private void AssertIsRoot()
{
if (Owner != null)
{
Expand All @@ -67,13 +68,13 @@ protected void AssertIsRoot()
}
}

protected void AssertIsNonFrozenRoot()
private void AssertIsNonFrozenRoot()
{
AssertNotFrozen();
AssertIsRoot();
}

protected void AssertIsAssignable(Characteristic characteristic, object value)
private static void AssertIsAssignable(Characteristic characteristic, object value)
{
if (ReferenceEquals(value, Characteristic.EmptyValue) || ReferenceEquals(value, null))
{
Expand All @@ -91,7 +92,8 @@ protected void AssertIsAssignable(Characteristic characteristic, object value)
#endregion

#region Properties
protected CharacteristicObject Owner => owner;

private CharacteristicObject Owner => owner;

protected CharacteristicObject OwnerOrSelf => owner ?? this;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace BenchmarkDotNet.Characteristics
using JetBrains.Annotations;

namespace BenchmarkDotNet.Characteristics
{
public abstract class CharacteristicObject<T> : CharacteristicObject
where T : CharacteristicObject<T>, new()
Expand All @@ -9,6 +11,7 @@ protected CharacteristicObject(string id) : base(id) { }

public new T Apply(CharacteristicObject other) => (T)ApplyCore(other);

[PublicAPI]
public T Apply(params CharacteristicObject[] others)
{
var result = this;
Expand Down
4 changes: 2 additions & 2 deletions src/BenchmarkDotNet/Characteristics/Characteristic`1.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ internal Characteristic(
FallbackValue = fallbackValue;
}

public Func<CharacteristicObject, T, T> Resolver { get; }
private Func<CharacteristicObject, T, T> Resolver { get; }

public new T FallbackValue { get; }
public T FallbackValue { get; }

public new T this[CharacteristicObject obj]
{
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Columns/BaselineScaledColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public string GetValue(Summary summary, BenchmarkCase benchmarkCase)
}
}

public bool IsNonBaselinesPrecise(Summary summary, Statistics baselineStat, BenchmarkCase benchmarkCase)
private static bool IsNonBaselinesPrecise(Summary summary, Statistics baselineStat, BenchmarkCase benchmarkCase)
{
string logicalGroupKey = summary.GetLogicalGroupKey(benchmarkCase);
var nonBaselines = summary.BenchmarksCases
Expand Down
11 changes: 6 additions & 5 deletions src/BenchmarkDotNet/Columns/DefaultColumnProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Mathematics;
using BenchmarkDotNet.Reports;
using JetBrains.Annotations;

namespace BenchmarkDotNet.Columns
{
public static class DefaultColumnProviders
{
public static readonly IColumnProvider Descriptor = new DescriptorColumnProvider();
public static readonly IColumnProvider Job = new JobColumnProvider();
public static readonly IColumnProvider Statistics = new StatisticsColumnProvider();
public static readonly IColumnProvider Params = new ParamsColumnProvider();
public static readonly IColumnProvider Diagnosers = new DiagnosersColumnProvider();
[PublicAPI] public static readonly IColumnProvider Descriptor = new DescriptorColumnProvider();
[PublicAPI] public static readonly IColumnProvider Job = new JobColumnProvider();
[PublicAPI] public static readonly IColumnProvider Statistics = new StatisticsColumnProvider();
[PublicAPI] public static readonly IColumnProvider Params = new ParamsColumnProvider();
[PublicAPI] public static readonly IColumnProvider Diagnosers = new DiagnosersColumnProvider();

public static readonly IColumnProvider[] Instance = { Descriptor, Job, Statistics, Params, Diagnosers };

Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Columns/JobCharacteristicColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class JobCharacteristicColumn : IColumn

private readonly Characteristic characteristic;

public JobCharacteristicColumn(Characteristic characteristic)
private JobCharacteristicColumn(Characteristic characteristic)
{
this.characteristic = characteristic;
Id = "Job." + characteristic.Id;
Expand Down
26 changes: 15 additions & 11 deletions src/BenchmarkDotNet/Columns/SizeUnit.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
using System.Linq;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using JetBrains.Annotations;

namespace BenchmarkDotNet.Columns
{
[SuppressMessage("ReSharper", "InconsistentNaming")] // We want to use "KB", "MB", "GB", "TB"
public class SizeUnit
{
public string Name { get; }
public string Description { get; }
public long ByteAmount { get; }
[PublicAPI] public string Name { get; }
[PublicAPI] public string Description { get; }
[PublicAPI] public long ByteAmount { get; }

public SizeUnit(string name, string description, long byteAmount)
{
Expand All @@ -15,13 +18,14 @@ public SizeUnit(string name, string description, long byteAmount)
ByteAmount = byteAmount;
}

public const long BytesInKiloByte = 1024L; // this value MUST NOT be changed
public static readonly SizeUnit B = new SizeUnit("B", "Byte", 1L);
public static readonly SizeUnit KB = new SizeUnit("KB", "Kilobyte", BytesInKiloByte);
public static readonly SizeUnit MB = new SizeUnit("MB", "Megabyte", BytesInKiloByte * BytesInKiloByte);
public static readonly SizeUnit GB = new SizeUnit("GB", "Gigabyte", BytesInKiloByte * BytesInKiloByte * BytesInKiloByte);
public static readonly SizeUnit TB = new SizeUnit("TB", "Terabyte", BytesInKiloByte * BytesInKiloByte * BytesInKiloByte * BytesInKiloByte);
public static readonly SizeUnit[] All = { B, KB, MB, GB, TB };
private const long BytesInKiloByte = 1024L; // this value MUST NOT be changed

[PublicAPI] public static readonly SizeUnit B = new SizeUnit("B", "Byte", 1L);
[PublicAPI] public static readonly SizeUnit KB = new SizeUnit("KB", "Kilobyte", BytesInKiloByte);
[PublicAPI] public static readonly SizeUnit MB = new SizeUnit("MB", "Megabyte", BytesInKiloByte * BytesInKiloByte);
[PublicAPI] public static readonly SizeUnit GB = new SizeUnit("GB", "Gigabyte", BytesInKiloByte * BytesInKiloByte * BytesInKiloByte);
[PublicAPI] public static readonly SizeUnit TB = new SizeUnit("TB", "Terabyte", BytesInKiloByte * BytesInKiloByte * BytesInKiloByte * BytesInKiloByte);
[PublicAPI] public static readonly SizeUnit[] All = { B, KB, MB, GB, TB };

public static SizeUnit GetBestSizeUnit(params long[] values)
{
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Columns/StatisticColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace BenchmarkDotNet.Columns
{
public class StatisticColumn : IColumn
{
public enum Priority
private enum Priority
{
Main,
Quartile,
Expand Down
16 changes: 7 additions & 9 deletions src/BenchmarkDotNet/Configs/ManualConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using BenchmarkDotNet.Order;
using BenchmarkDotNet.Reports;
using BenchmarkDotNet.Validators;
using JetBrains.Annotations;

namespace BenchmarkDotNet.Configs
{
Expand Down Expand Up @@ -43,15 +44,11 @@ public class ManualConfig : IConfig

public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => logicalGroupRules;

public ConfigUnionRule UnionRule { get; set; } = ConfigUnionRule.Union;

public bool KeepBenchmarkFiles { get; set; }

public bool SummaryPerType { get; set; } = true;

public string ArtifactsPath { get; set; }

public Encoding Encoding { get; set; }
[PublicAPI] public ConfigUnionRule UnionRule { get; set; } = ConfigUnionRule.Union;
[PublicAPI] public bool KeepBenchmarkFiles { get; set; }
[PublicAPI] public bool SummaryPerType { get; set; } = true;
[PublicAPI] public string ArtifactsPath { get; set; }
[PublicAPI] public Encoding Encoding { get; set; }

public void Add(params IColumn[] newColumns) => columnProviders.AddRange(newColumns.Select(c => c.ToProvider()));
public void Add(params IColumnProvider[] newColumnProviders) => columnProviders.AddRange(newColumnProviders);
Expand All @@ -68,6 +65,7 @@ public class ManualConfig : IConfig
public void Set(Encoding encoding) => Encoding = encoding;
public void Add(params BenchmarkLogicalGroupRule[] rules) => logicalGroupRules.AddRange(rules);

[PublicAPI]
public void Add(IConfig config)
{
columnProviders.AddRange(config.GetColumnProviders());
Expand Down
Loading

0 comments on commit a76f438

Please sign in to comment.