Skip to content

Commit

Permalink
Fix dotnet#1715 with the ability to revert the old behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
YegorStepanov committed Nov 1, 2022
1 parent a15545b commit b703454
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 12 deletions.
13 changes: 11 additions & 2 deletions src/BenchmarkDotNet/Reports/SummaryStyle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ public class SummaryStyle : IEquatable<SummaryStyle>

public RatioStyle RatioStyle { get; }

public bool OldBehaviorForBenchmarkName { get; }

public SummaryStyle([CanBeNull] CultureInfo cultureInfo, bool printUnitsInHeader, SizeUnit sizeUnit, TimeUnit timeUnit, bool printUnitsInContent = true,
bool printZeroValuesInContent = false, int maxParameterColumnWidth = DefaultMaxParameterColumnWidth, RatioStyle ratioStyle = RatioStyle.Value)
bool printZeroValuesInContent = false, int maxParameterColumnWidth = DefaultMaxParameterColumnWidth, RatioStyle ratioStyle = RatioStyle.Value,
bool oldBehaviorForBenchmarkName = false)
{
if (maxParameterColumnWidth < DefaultMaxParameterColumnWidth)
throw new ArgumentOutOfRangeException(nameof(maxParameterColumnWidth), $"{DefaultMaxParameterColumnWidth} is the minimum.");
Expand All @@ -41,6 +44,7 @@ public SummaryStyle([CanBeNull] CultureInfo cultureInfo, bool printUnitsInHeader
MaxParameterColumnWidth = maxParameterColumnWidth;
RatioStyle = ratioStyle;
CodeSizeUnit = SizeUnit.B;
OldBehaviorForBenchmarkName = oldBehaviorForBenchmarkName;
}

public SummaryStyle WithTimeUnit(TimeUnit timeUnit)
Expand All @@ -61,6 +65,9 @@ public SummaryStyle WithCultureInfo(CultureInfo cultureInfo)
public SummaryStyle WithRatioStyle(RatioStyle ratioStyle)
=> new SummaryStyle(CultureInfo, PrintUnitsInHeader, SizeUnit, TimeUnit, PrintUnitsInContent, PrintZeroValuesInContent, MaxParameterColumnWidth, ratioStyle);

public SummaryStyle WithOldBehaviorForBenchmarkName(bool oldBehaviorForBenchmarkName)
=> new SummaryStyle(CultureInfo, PrintUnitsInHeader, SizeUnit, TimeUnit, PrintUnitsInContent, PrintZeroValuesInContent, MaxParameterColumnWidth, RatioStyle, oldBehaviorForBenchmarkName);

public bool Equals(SummaryStyle other)
{
if (ReferenceEquals(null, other))
Expand All @@ -75,7 +82,8 @@ public bool Equals(SummaryStyle other)
&& Equals(CodeSizeUnit, other.CodeSizeUnit)
&& Equals(TimeUnit, other.TimeUnit)
&& MaxParameterColumnWidth == other.MaxParameterColumnWidth
&& RatioStyle == other.RatioStyle;
&& RatioStyle == other.RatioStyle
&& OldBehaviorForBenchmarkName == other.OldBehaviorForBenchmarkName;
}

public override bool Equals(object obj) => obj is SummaryStyle summary && Equals(summary);
Expand All @@ -92,6 +100,7 @@ public override int GetHashCode()
hashCode = (hashCode * 397) ^ (TimeUnit != null ? TimeUnit.GetHashCode() : 0);
hashCode = (hashCode * 397) ^ MaxParameterColumnWidth;
hashCode = (hashCode * 397) ^ RatioStyle.GetHashCode();
hashCode = (hashCode * 397) ^ OldBehaviorForBenchmarkName.GetHashCode();
return hashCode;
}
}
Expand Down
14 changes: 9 additions & 5 deletions src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private static BenchmarkRunInfo MethodsToBenchmarksWithFullConfig(Type type, Met
var iterationSetupMethods = GetAttributedMethods<IterationSetupAttribute>(allMethods, "IterationSetup");
var iterationCleanupMethods = GetAttributedMethods<IterationCleanupAttribute>(allMethods, "IterationCleanup");

var targets = GetTargets(benchmarkMethods, type, globalSetupMethods, globalCleanupMethods, iterationSetupMethods, iterationCleanupMethods).ToArray();
var targets = GetTargets(benchmarkMethods, type, globalSetupMethods, globalCleanupMethods, iterationSetupMethods, iterationCleanupMethods, configPerType.SummaryStyle.OldBehaviorForBenchmarkName).ToArray();

var parameterDefinitions = GetParameterDefinitions(type);
var parameterInstancesList = parameterDefinitions.Expand(configPerType.SummaryStyle);
Expand Down Expand Up @@ -115,7 +115,8 @@ private static IEnumerable<Descriptor> GetTargets(
Tuple<MethodInfo, TargetedAttribute>[] globalSetupMethods,
Tuple<MethodInfo, TargetedAttribute>[] globalCleanupMethods,
Tuple<MethodInfo, TargetedAttribute>[] iterationSetupMethods,
Tuple<MethodInfo, TargetedAttribute>[] iterationCleanupMethods)
Tuple<MethodInfo, TargetedAttribute>[] iterationCleanupMethods,
bool oldBehaviorForBenchmarkName)
{
return targetMethods
.Select(methodInfo => CreateDescriptor(type,
Expand All @@ -125,7 +126,8 @@ private static IEnumerable<Descriptor> GetTargets(
GetTargetedMatchingMethod(methodInfo, iterationSetupMethods),
GetTargetedMatchingMethod(methodInfo, iterationCleanupMethods),
methodInfo.ResolveAttribute<BenchmarkAttribute>(),
targetMethods));
targetMethods,
oldBehaviorForBenchmarkName));
}

private static MethodInfo GetTargetedMatchingMethod(MethodInfo benchmarkMethod, Tuple<MethodInfo, TargetedAttribute>[] methods)
Expand All @@ -152,7 +154,8 @@ private static Descriptor CreateDescriptor(
MethodInfo iterationSetupMethod,
MethodInfo iterationCleanupMethod,
BenchmarkAttribute attr,
MethodInfo[] targetMethods)
MethodInfo[] targetMethods,
bool oldBehaviorForBenchmarkName)
{
var target = new Descriptor(
type,
Expand All @@ -165,7 +168,8 @@ private static Descriptor CreateDescriptor(
baseline: attr.Baseline,
categories: GetCategories(methodInfo),
operationsPerInvoke: attr.OperationsPerInvoke,
methodIndex: Array.IndexOf(targetMethods, methodInfo));
methodIndex: Array.IndexOf(targetMethods, methodInfo),
oldBehaviorForBenchmarkName: oldBehaviorForBenchmarkName);
AssertMethodHasCorrectSignature("Benchmark", methodInfo);
AssertMethodIsAccessible("Benchmark", methodInfo);
AssertMethodIsNotGeneric("Benchmark", methodInfo);
Expand Down
3 changes: 2 additions & 1 deletion src/BenchmarkDotNet/Running/ClassicBenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ public static BenchmarkRunInfo[] SourceToBenchmarks(string source, IConfig confi
return BenchmarkCase.Create(
new Descriptor(target.Type, target.WorkloadMethod, target.GlobalSetupMethod, target.GlobalCleanupMethod,
target.IterationSetupMethod, target.IterationCleanupMethod,
target.WorkloadMethodDisplayInfo, benchmarkContent, target.Baseline, target.Categories, target.OperationsPerInvoke),
target.WorkloadMethodDisplayInfo, benchmarkContent, target.Baseline, target.Categories, target.OperationsPerInvoke,
oldBehaviorForBenchmarkName: config?.SummaryStyle?.OldBehaviorForBenchmarkName ?? false),
b.Job,
b.Parameters,
b.Config);
Expand Down
10 changes: 7 additions & 3 deletions src/BenchmarkDotNet/Running/Descriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public Descriptor(
bool baseline = false,
string[] categories = null,
int operationsPerInvoke = 1,
int methodIndex = 0)
int methodIndex = 0,
bool oldBehaviorForBenchmarkName = false)
{
Type = type;
WorkloadMethod = workloadMethod;
Expand All @@ -51,16 +52,19 @@ public Descriptor(
IterationCleanupMethod = iterationCleanupMethod;
OperationsPerInvoke = operationsPerInvoke;
AdditionalLogic = additionalLogic ?? string.Empty;
WorkloadMethodDisplayInfo = FormatDescription(description) ?? workloadMethod?.Name ?? "Untitled";
WorkloadMethodDisplayInfo = FormatDescription(description, oldBehaviorForBenchmarkName) ?? workloadMethod?.Name ?? "Untitled";
Baseline = baseline;
Categories = categories ?? Array.Empty<string>();
MethodIndex = methodIndex;
}

public override string ToString() => DisplayInfo;

private static string FormatDescription([CanBeNull] string description)
private static string FormatDescription([CanBeNull] string description, bool oldBehaviorForBenchmarkName)
{
if (!oldBehaviorForBenchmarkName)
return description;

var specialSymbols = new[] { ' ', '\'', '[', ']' };
return description != null && specialSymbols.Any(description.Contains)
? "'" + description + "'"
Expand Down
38 changes: 37 additions & 1 deletion tests/BenchmarkDotNet.Tests/Running/BenchmarkConverterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Reports;
using BenchmarkDotNet.Running;
using Perfolizer.Mathematics.OutlierDetection;
using Xunit;
Expand Down Expand Up @@ -278,5 +279,40 @@ public class PrivateIterationCleanup
[IterationCleanup] private void X() { }
[Benchmark] public void A() { }
}

[Fact]
public void BenchmarkNameWithSpecialCharactersIsNotEscapedByDefault()
{
var info = BenchmarkConverter.TypeToBenchmarks(typeof(BenchmarkNamesWithSpecialCharacters));

Assert.Equal("with space", info.BenchmarksCases[0].Descriptor.WorkloadMethodDisplayInfo);
Assert.Equal("na\'me", info.BenchmarksCases[1].Descriptor.WorkloadMethodDisplayInfo);
Assert.Equal("[name]", info.BenchmarksCases[2].Descriptor.WorkloadMethodDisplayInfo);
}

[Fact]
public void OldBenchmarkNameBehaviorCanBeEnabledWithSummaryStyle()
{
var config = DefaultConfig.Instance.WithSummaryStyle(
SummaryStyle.Default.WithOldBehaviorForBenchmarkName(true));

var info = BenchmarkConverter.TypeToBenchmarks(typeof(BenchmarkNamesWithSpecialCharacters), config);

Assert.Equal("'with space'", info.BenchmarksCases[0].Descriptor.WorkloadMethodDisplayInfo);
Assert.Equal("'na\'me'", info.BenchmarksCases[1].Descriptor.WorkloadMethodDisplayInfo);
Assert.Equal("'[name]'", info.BenchmarksCases[2].Descriptor.WorkloadMethodDisplayInfo);
}

public sealed class BenchmarkNamesWithSpecialCharacters
{
[Benchmark(Description = "with space")]
public void A() { }

[Benchmark(Description = "na\'me")]
public void B() { }

[Benchmark(Description = "[name]")]
public void C() { }
}
}
}
}

0 comments on commit b703454

Please sign in to comment.