Skip to content

Commit 81ac4c9

Browse files
authored
Fix duplicated test name in Perf.TypeName and update validation (#4898)
1 parent 74dd256 commit 81ac4c9

File tree

3 files changed

+40
-25
lines changed

3 files changed

+40
-25
lines changed

src/benchmarks/micro/libraries/System.Reflection.Metadata/Perf.TypeName.cs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,40 +11,53 @@ namespace System.Reflection.Metadata
1111
[BenchmarkCategory(Categories.Libraries)]
1212
public class Perf_TypeName
1313
{
14+
public class ArgumentTypeWrapper
15+
{
16+
private readonly string displayName;
17+
18+
public ArgumentTypeWrapper(Type type, string displayName)
19+
{
20+
Type = type;
21+
this.displayName = displayName;
22+
}
23+
24+
public Type Type { get; }
25+
public override string ToString() => displayName;
26+
}
27+
1428
public IEnumerable<object> TypeArguments()
1529
{
16-
// We don't return strings here, as they change over the time when assembly versions get increased.
17-
// This would change benchmark ID and loose historical data tracking.
18-
yield return typeof(int); // elemental type
19-
yield return typeof(int).MakePointerType(); // a pointer to an elemental type
20-
yield return typeof(int).MakeByRefType(); // a reference to an elemental type
21-
yield return typeof(int[]); // SZArray
22-
yield return typeof(int).MakeArrayType(1); // single-dimensional array, but not indexed from 0
23-
yield return typeof(int).MakeArrayType(2); // multi-dimensional array
24-
yield return typeof(Dictionary<string, bool>); // generic type
25-
yield return typeof(Dictionary<string, bool>[]); // an array of generic types
26-
yield return typeof(Nested); // nested type
27-
yield return typeof(Nested.NestedGeneric<string, bool>); // nested generic type
28-
yield return typeof(Dictionary<List<int[]>[,], List<int?[][][,]>>[]); // complex generic type (node count = 16)
30+
// We use a wrapper type for each argument to ensure that the test name remains the same into the future
31+
yield return new ArgumentTypeWrapper(typeof(int), "typeof(int)"); // elemental type
32+
yield return new ArgumentTypeWrapper(typeof(int).MakePointerType(), "typeof(System.Int32*)"); // a pointer to an elemental type
33+
yield return new ArgumentTypeWrapper(typeof(int).MakeByRefType(), "typeof(System.Int32&)"); // a reference to an elemental type
34+
yield return new ArgumentTypeWrapper(typeof(int[]), "typeof(System.Int32[])"); // SZArray
35+
yield return new ArgumentTypeWrapper(typeof(int).MakeArrayType(1), "typeof(System.Int32[*])"); // single-dimensional array, but not indexed from 0
36+
yield return new ArgumentTypeWrapper(typeof(int).MakeArrayType(2), "typeof(System.Int32[,])"); // multi-dimensional array
37+
yield return new ArgumentTypeWrapper(typeof(Dictionary<string, bool>), "typeof(System.Collections.Generic.Dictionary<String, Boolean>)"); // generic type
38+
yield return new ArgumentTypeWrapper(typeof(Dictionary<string, bool>[]), "typeof(System.Collections.Generic.Dictionary`2[])"); // an array of generic types
39+
yield return new ArgumentTypeWrapper(typeof(Nested), "typeof(System.Reflection.Metadata.Nested)"); // nested type
40+
yield return new ArgumentTypeWrapper(typeof(Nested.NestedGeneric<string, bool>), "typeof(System.Reflection.Metadata.NestedGeneric<String, Boolean>)"); // nested generic type
41+
yield return new ArgumentTypeWrapper(typeof(Dictionary<List<int[]>[,], List<int?[][][,]>>[]), "typeof(System.Collections.Generic.Dictionary`2[]) (COMPLEX)"); // complex generic type (node count = 16)
2942
}
3043

3144
[Benchmark]
3245
[ArgumentsSource(nameof(TypeArguments))]
33-
public TypeName Parse_FullNames(Type input) => TypeName.Parse(input.FullName);
46+
public TypeName Parse_FullNames(ArgumentTypeWrapper input) => TypeName.Parse(input.Type.FullName);
3447

3548
[Benchmark]
3649
[ArgumentsSource(nameof(TypeArguments))]
37-
public TypeName Parse_AssemblyQualifiedName(Type input) => TypeName.Parse(input.AssemblyQualifiedName);
50+
public TypeName Parse_AssemblyQualifiedName(ArgumentTypeWrapper input) => TypeName.Parse(input.Type.AssemblyQualifiedName);
3851

3952
// The Name, FullName and AssemblyQualifiedName properties are lazy and cached,
40-
// so we need to parse a new TypName instance in order to get these properties calculated.
53+
// so we need to parse a new TypeName instance in order to get these properties calculated.
4154
[Benchmark]
4255
[ArgumentsSource(nameof(TypeArguments))]
43-
public string ParseAndGetFullName(Type input) => TypeName.Parse(input.FullName).FullName;
56+
public string ParseAndGetFullName(ArgumentTypeWrapper input) => TypeName.Parse(input.Type.FullName).FullName;
4457

4558
[Benchmark]
4659
[ArgumentsSource(nameof(TypeArguments))]
47-
public string ParseAndGetAssemblyQualifiedName(Type input) => TypeName.Parse(input.AssemblyQualifiedName).AssemblyQualifiedName;
60+
public string ParseAndGetAssemblyQualifiedName(ArgumentTypeWrapper input) => TypeName.Parse(input.Type.AssemblyQualifiedName).AssemblyQualifiedName;
4861

4962
public IEnumerable<string> InvalidArguments()
5063
{

src/harness/BenchmarkDotNet.Extensions/UniqueArgumentsValidator.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.Generic;
77
using System.Linq;
88
using BenchmarkDotNet.Running;
9+
using BenchmarkDotNet.Exporters;
910

1011
namespace BenchmarkDotNet.Extensions
1112
{
@@ -29,14 +30,17 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara
2930
private class BenchmarkArgumentsComparer : IEqualityComparer<BenchmarkCase>
3031
{
3132
public bool Equals(BenchmarkCase x, BenchmarkCase y)
32-
=> Enumerable.SequenceEqual(
33+
{
34+
if (FullNameProvider.GetBenchmarkName(x).Equals(FullNameProvider.GetBenchmarkName(y), System.StringComparison.Ordinal))
35+
return true;
36+
37+
return Enumerable.SequenceEqual(
3338
x.Parameters.Items.Select(argument => argument.Value),
3439
y.Parameters.Items.Select(argument => argument.Value));
40+
}
3541

3642
public int GetHashCode(BenchmarkCase obj)
37-
=> obj.Parameters.Items
38-
.Where(item => item.Value != null)
39-
.Aggregate(seed: 0, (hashCode, argument) => hashCode ^= argument.Value.GetHashCode());
43+
=> FullNameProvider.GetBenchmarkName(obj).GetHashCode();
4044
}
4145
}
4246
}

src/tools/Reporting/Reporting/Reporter.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ public void AddTest(Test test)
5959
{
6060
if (Tests.Any(t => t.Name.Equals(test.Name)))
6161
{
62-
// TODO: Make this surface as a failure so it gets fixed, this is just temporary to fix the pipeline
63-
Console.Error.WriteLine($"Duplicate test name: {test.Name}, skipping from results");
64-
return;
62+
throw new Exception($"Duplicate test name: {test.Name} found");
6563
}
6664

6765
Tests.Add(test);

0 commit comments

Comments
 (0)