Skip to content

Commit de52812

Browse files
User0332timcassell
User0332
and
timcassell
authored
Allow benchmark methods to use names that are used by code generation templates (fixes #2821) (#2897)
* Allow benchmark methods to use names that appear in source generation templates * Use global:: prefix for setup, cleanup, and async benchmarks. Fix missing prefixes. Update Params and Arguments. Remove ValidateNamingConflicts and test more name conflicts. * Fix SmartParam and SmartArgument. * Fix static param. * Re-add sealed modifier. * Fix InProcessBenchmarkEmitsSameIL test. * Update comments. --------- Co-authored-by: Tim Cassell <cassell.timothy@gmail.com>
1 parent 6b77c83 commit de52812

File tree

15 files changed

+196
-146
lines changed

15 files changed

+196
-146
lines changed

‎src/BenchmarkDotNet/Characteristics/CharacteristicPresenter.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public override string ToPresentation(object? characteristicValue, Characteristi
8383
{
8484
// TODO: DO NOT hardcode Characteristic suffix
8585
string id = characteristic.Id;
86-
string type = characteristic.DeclaringType.FullName!;
86+
string type = characteristic.DeclaringType.GetCorrectCSharpTypeName();
8787
string value = SourceCodeHelper.ToSourceCode(characteristicValue);
8888
return $"{type}.{id}Characteristic[job] = {value}";
8989
}

‎src/BenchmarkDotNet/Code/CodeGenerator.cs‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ internal static string Generate(BuildPartition buildPartition)
4949
.Replace("$IterationSetupMethodName$", provider.IterationSetupMethodName)
5050
.Replace("$IterationCleanupMethodName$", provider.IterationCleanupMethodName)
5151
.Replace("$JobSetDefinition$", GetJobsSetDefinition(benchmark))
52-
.Replace("$ParamsInitializer$", GetParamsInitializer(benchmark))
5352
.Replace("$ParamsContent$", GetParamsContent(benchmark))
5453
.Replace("$ArgumentsDefinition$", GetArgumentsDefinition(benchmark))
5554
.Replace("$DeclareArgumentFields$", GetDeclareArgumentFields(benchmark))
56-
.Replace("$InitializeArgumentFields$", GetInitializeArgumentFields(benchmark)).Replace("$LoadArguments$", GetLoadArguments(benchmark))
55+
.Replace("$InitializeArgumentFields$", GetInitializeArgumentFields(benchmark))
56+
.Replace("$LoadArguments$", GetLoadArguments(benchmark))
5757
.Replace("$PassArguments$", passArguments)
5858
.Replace("$EngineFactoryType$", GetEngineFactoryTypeName(benchmark))
5959
.Replace("$MeasureExtraStats$", buildInfo.Config.HasExtraStatsDiagnoser() ? "true" : "false")
@@ -159,21 +159,14 @@ private static DeclarationsProvider GetDeclarationsProvider(Descriptor descripto
159159
return new SyncDeclarationsProvider(descriptor);
160160
}
161161

162-
private static string GetParamsInitializer(BenchmarkCase benchmarkCase)
163-
=> string.Join(
164-
", ",
165-
benchmarkCase.Parameters.Items
166-
.Where(parameter => !parameter.IsArgument && !parameter.IsStatic)
167-
.Select(parameter => $"{parameter.Name} = default"));
168-
169162
// internal for tests
170163

171164
internal static string GetParamsContent(BenchmarkCase benchmarkCase)
172165
=> string.Join(
173166
string.Empty,
174167
benchmarkCase.Parameters.Items
175168
.Where(parameter => !parameter.IsArgument)
176-
.Select(parameter => $"{(parameter.IsStatic ? "" : "instance.")}{parameter.Name} = {parameter.ToSourceCode()};"));
169+
.Select(parameter => $"{(parameter.IsStatic ? benchmarkCase.Descriptor.Type.GetCorrectCSharpTypeName() : "base")}.{parameter.Name} = {parameter.ToSourceCode()};"));
177170

178171
private static string GetArgumentsDefinition(BenchmarkCase benchmarkCase)
179172
=> string.Join(
@@ -191,13 +184,13 @@ private static string GetInitializeArgumentFields(BenchmarkCase benchmarkCase)
191184
=> string.Join(
192185
Environment.NewLine,
193186
benchmarkCase.Descriptor.WorkloadMethod.GetParameters()
194-
.Select((parameter, index) => $"__argField{index} = {benchmarkCase.Parameters.GetArgument(parameter.Name).ToSourceCode()};")); // we init the fields in ctor to provoke all possible allocations and overhead of other type
187+
.Select((parameter, index) => $"this.__argField{index} = {benchmarkCase.Parameters.GetArgument(parameter.Name).ToSourceCode()};")); // we init the fields in ctor to provoke all possible allocations and overhead of other type
195188

196189
private static string GetLoadArguments(BenchmarkCase benchmarkCase)
197190
=> string.Join(
198191
Environment.NewLine,
199192
benchmarkCase.Descriptor.WorkloadMethod.GetParameters()
200-
.Select((parameter, index) => $"{(parameter.ParameterType.IsByRef ? "ref" : string.Empty)} {parameter.ParameterType.GetCorrectCSharpTypeName()} arg{index} = {(parameter.ParameterType.IsByRef ? "ref" : string.Empty)} __argField{index};"));
193+
.Select((parameter, index) => $"{(parameter.ParameterType.IsByRef ? "ref" : string.Empty)} {parameter.ParameterType.GetCorrectCSharpTypeName()} arg{index} = {(parameter.ParameterType.IsByRef ? "ref" : string.Empty)} this.__argField{index};"));
201194

202195
private static string GetPassArguments(BenchmarkCase benchmarkCase)
203196
=> string.Join(

‎src/BenchmarkDotNet/Code/DeclarationsProvider.cs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ internal abstract class DeclarationsProvider
2626

2727
public string IterationCleanupMethodName => Descriptor.IterationCleanupMethod?.Name ?? EmptyAction;
2828

29-
public virtual string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments})";
29+
public abstract string GetWorkloadMethodCall(string passArguments);
30+
31+
protected static string GetMethodPrefix(MethodInfo method)
32+
=> method.IsStatic ? method.DeclaringType.GetCorrectCSharpTypeName() : "base";
3033

3134
private string GetMethodName(MethodInfo method)
3235
{
@@ -41,22 +44,22 @@ private string GetMethodName(MethodInfo method)
4144
(method.ReturnType.GetGenericTypeDefinition() == typeof(Task<>) ||
4245
method.ReturnType.GetGenericTypeDefinition() == typeof(ValueTask<>))))
4346
{
44-
return $"() => BenchmarkDotNet.Helpers.AwaitHelper.GetResult({method.Name}())";
47+
return $"() => global::BenchmarkDotNet.Helpers.AwaitHelper.GetResult({GetMethodPrefix(Descriptor.WorkloadMethod)}.{method.Name}())";
4548
}
4649

47-
return method.Name;
50+
return $"{GetMethodPrefix(Descriptor.WorkloadMethod)}.{method.Name}";
4851
}
4952
}
5053

51-
internal class SyncDeclarationsProvider : DeclarationsProvider
54+
internal class SyncDeclarationsProvider(Descriptor descriptor) : DeclarationsProvider(descriptor)
5255
{
53-
public SyncDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }
56+
public override string GetWorkloadMethodCall(string passArguments)
57+
=> $"{GetMethodPrefix(Descriptor.WorkloadMethod)}.{Descriptor.WorkloadMethod.Name}({passArguments})";
5458
}
5559

56-
internal class AsyncDeclarationsProvider : DeclarationsProvider
60+
internal class AsyncDeclarationsProvider(Descriptor descriptor) : DeclarationsProvider(descriptor)
5761
{
58-
public AsyncDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }
59-
60-
public override string GetWorkloadMethodCall(string passArguments) => $"BenchmarkDotNet.Helpers.AwaitHelper.GetResult({Descriptor.WorkloadMethod.Name}({passArguments}))";
62+
public override string GetWorkloadMethodCall(string passArguments)
63+
=> $"global::BenchmarkDotNet.Helpers.AwaitHelper.GetResult({GetMethodPrefix(Descriptor.WorkloadMethod)}.{Descriptor.WorkloadMethod.Name}({passArguments}))";
6164
}
6265
}

‎src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Linq;
44
using System.Reflection;
5+
using System.Security.Cryptography.X509Certificates;
56
using BenchmarkDotNet.Attributes;
67

78
namespace BenchmarkDotNet.Extensions
@@ -36,7 +37,7 @@ public static bool IsInitOnly(this PropertyInfo propertyInfo)
3637
/// <summary>
3738
/// returns type name which can be used in generated C# code
3839
/// </summary>
39-
internal static string GetCorrectCSharpTypeName(this Type type, bool includeNamespace = true, bool includeGenericArgumentsNamespace = true)
40+
internal static string GetCorrectCSharpTypeName(this Type type, bool includeNamespace = true, bool includeGenericArgumentsNamespace = true, bool prefixWithGlobal = true)
4041
{
4142
while (!(type.IsPublic || type.IsNestedPublic) && type.BaseType != null)
4243
type = type.BaseType;
@@ -49,16 +50,23 @@ internal static string GetCorrectCSharpTypeName(this Type type, bool includeName
4950
return "void";
5051
if (type == typeof(void*))
5152
return "void*";
53+
5254
string prefix = "";
55+
5356
if (!string.IsNullOrEmpty(type.Namespace) && includeNamespace)
57+
{
5458
prefix += type.Namespace + ".";
5559

60+
if (prefixWithGlobal)
61+
prefix = $"global::{prefix}";
62+
}
63+
5664
if (type.GetTypeInfo().IsGenericParameter)
5765
return type.Name;
5866

5967
if (type.IsArray)
6068
{
61-
var typeName = GetCorrectCSharpTypeName(type.GetElementType());
69+
var typeName = GetCorrectCSharpTypeName(type.GetElementType(), includeNamespace, includeGenericArgumentsNamespace, prefixWithGlobal);
6270
var parts = typeName.Split(['['], count: 2);
6371

6472
string repr = parts[0] + '[' + new string(',', type.GetArrayRank() - 1) + ']';
@@ -68,11 +76,11 @@ internal static string GetCorrectCSharpTypeName(this Type type, bool includeName
6876
return repr;
6977
}
7078

71-
return prefix + string.Join(".", GetNestedTypeNames(type, includeGenericArgumentsNamespace).Reverse());
79+
return prefix + string.Join(".", GetNestedTypeNames(type, includeGenericArgumentsNamespace, prefixWithGlobal).Reverse());
7280
}
7381

7482
// from most nested to least
75-
private static IEnumerable<string> GetNestedTypeNames(Type type, bool includeGenericArgumentsNamespace)
83+
private static IEnumerable<string> GetNestedTypeNames(Type type, bool includeGenericArgumentsNamespace, bool prefixWithGlobal)
7684
{
7785
var allTypeParameters = new Stack<Type>(type.GetGenericArguments());
7886

@@ -92,7 +100,7 @@ private static IEnumerable<string> GetNestedTypeNames(Type type, bool includeGen
92100
.Select(_ => allTypeParameters.Pop())
93101
.Reverse();
94102

95-
var args = string.Join(", ", typeParameters.Select(T => GetCorrectCSharpTypeName(T, includeGenericArgumentsNamespace, includeGenericArgumentsNamespace)));
103+
var args = string.Join(", ", typeParameters.Select(T => GetCorrectCSharpTypeName(T, includeGenericArgumentsNamespace, includeGenericArgumentsNamespace, prefixWithGlobal)));
96104
name = $"{mainName}<{args}>";
97105
}
98106

‎src/BenchmarkDotNet/Helpers/FolderNameHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public static string ToFolderName(object? value)
4444
// we can't simply use type.FullName, because for generics it's too long
4545
// example: typeof(List<int>).FullName => "System.Collections.Generic.List`1[[System.Int32, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]"
4646
public static string ToFolderName(Type type, bool includeNamespace = true, bool includeGenericArgumentsNamespace = false)
47-
=> Escape(new StringBuilder(type.GetCorrectCSharpTypeName(includeNamespace, includeGenericArgumentsNamespace)));
47+
=> Escape(new StringBuilder(type.GetCorrectCSharpTypeName(includeNamespace, includeGenericArgumentsNamespace, prefixWithGlobal: false)));
4848

4949
private static string Escape(StringBuilder builder)
5050
{

‎src/BenchmarkDotNet/Parameters/SmartParamBuilder.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public string ToSourceCode()
125125
else
126126
{
127127
// If the source member is non-static, we mustn't include the type name, as this would be a compiler error when accessing a non-static source member in the base class of this generated type.
128-
methodCall = source.Name;
128+
methodCall = $"base.{source.Name}";
129129
}
130130

131131
// we do something like enumerable.ElementAt(sourceIndex)[argumentIndex];
@@ -157,12 +157,12 @@ public string ToSourceCode()
157157
{
158158
string cast = $"({parameterType.GetCorrectCSharpTypeName()})"; // it's an object so we need to cast it to the right type
159159

160-
string instancePrefix = method.IsStatic ? source.DeclaringType.GetCorrectCSharpTypeName() : "instance";
160+
string callPrefix = method.IsStatic ? source.DeclaringType.GetCorrectCSharpTypeName() : "base";
161161

162162
string callPostfix = source is PropertyInfo ? string.Empty : "()";
163163

164164
// we so something like enumerable.ElementAt(index);
165-
return $"{cast}BenchmarkDotNet.Parameters.ParameterExtractor.GetParameter({instancePrefix}.{source.Name}{callPostfix}, {index});";
165+
return $"{cast}BenchmarkDotNet.Parameters.ParameterExtractor.GetParameter({callPrefix}.{source.Name}{callPostfix}, {index});";
166166
}
167167
}
168168

‎src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ internal static Summary[] Run(BenchmarkRunInfo[] benchmarkRunInfos)
137137
var benchmarkWithHighestIdForGivenType = benchmarkRunInfo.BenchmarksCases.Last();
138138
if (benchmarkToBuildResult[benchmarkWithHighestIdForGivenType].Id.Value <= idToResume)
139139
{
140-
compositeLogger.WriteLineInfo($"Skipping {benchmarkRunInfo.BenchmarksCases.Length} benchmark(s) defined by {benchmarkRunInfo.Type.GetCorrectCSharpTypeName()}.");
140+
compositeLogger.WriteLineInfo($"Skipping {benchmarkRunInfo.BenchmarksCases.Length} benchmark(s) defined by {benchmarkRunInfo.Type.GetCorrectCSharpTypeName(prefixWithGlobal: false)}.");
141141
continue;
142142
}
143143
}

‎src/BenchmarkDotNet/Running/Descriptor.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public Descriptor(
7171

7272
public bool HasCategory(string category) => Categories.Any(c => c.EqualsWithIgnoreCase(category));
7373

74-
public string GetFilterName() => $"{Type.GetCorrectCSharpTypeName(includeGenericArgumentsNamespace: false)}.{WorkloadMethod.Name}";
74+
public string GetFilterName() => $"{Type.GetCorrectCSharpTypeName(includeGenericArgumentsNamespace: false, prefixWithGlobal: false)}.{WorkloadMethod.Name}";
7575

7676
public bool Equals(Descriptor? other) => GetFilterName().Equals(other?.GetFilterName());
7777

0 commit comments

Comments
 (0)