Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework ReturnValueValidator and ExecutionValidator #2114

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,31 +156,27 @@ private static MethodInfo[] GetBenchmarks(this TypeInfo typeInfo)
.Where(method => method.GetCustomAttributes(true).OfType<BenchmarkAttribute>().Any())
.ToArray();

internal static (string Name, TAttribute Attribute, bool IsPrivate, bool IsStatic, Type ParameterType)[] GetTypeMembersWithGivenAttribute<TAttribute>(this Type type, BindingFlags reflectionFlags)
internal static (string Name, TAttribute Attribute, bool IsPublic, bool IsStatic, Type ParameterType)[] GetTypeMembersWithGivenAttribute<TAttribute>(this Type type, BindingFlags reflectionFlags)
where TAttribute : Attribute
{
var allFields = type.GetFields(reflectionFlags)
.Select(f => (
Name: f.Name,
Attribute: f.ResolveAttribute<TAttribute>(),
IsPrivate: f.IsPrivate,
IsPublic: f.IsPublic,
IsStatic: f.IsStatic,
ParameterType: f.FieldType));

var allProperties = type.GetProperties(reflectionFlags)
.Select(p => (
Name: p.Name,
Attribute: p.ResolveAttribute<TAttribute>(),
IsPrivate: p.GetSetMethod() == null,
IsPublic: p.GetSetMethod() != null && p.GetSetMethod().IsPublic,
IsStatic: p.GetSetMethod() != null && p.GetSetMethod().IsStatic,
PropertyType: p.PropertyType));

var joined = allFields.Concat(allProperties).Where(member => member.Attribute != null).ToArray();

foreach (var member in joined.Where(m => m.IsPrivate))
throw new InvalidOperationException($"Member \"{member.Name}\" must be public if it has the [{typeof(TAttribute).Name}] attribute applied to it");

return joined;
var joined = allFields.Concat(allProperties).Where(member => member.Attribute != null);
return joined.ToArray();
}

internal static bool IsStackOnlyWithImplicitCast(this Type argumentType, object argumentInstance)
Expand Down
4 changes: 4 additions & 0 deletions src/BenchmarkDotNet/Running/BenchmarkConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ IEnumerable<ParameterDefinition> GetDefinitions<TAttribute>(Func<TAttribute, Typ
const BindingFlags reflectionFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;

var allMembers = type.GetTypeMembersWithGivenAttribute<TAttribute>(reflectionFlags);

foreach (var member in allMembers.Where(m => !m.IsPublic))
throw new InvalidOperationException($"Member \"{member.Name}\" must be public if it has the [{typeof(TAttribute).Name}] attribute applied to it");

return allMembers.Select(member =>
new ParameterDefinition(
member.Name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Exporters;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Parameters;
using BenchmarkDotNet.Running;

using JetBrains.Annotations;
Expand Down Expand Up @@ -61,39 +62,43 @@ public static int Run(IHost host, BenchmarkCase benchmarkCase)
}
}

/// <summary>Fills the properties of the instance of the object used to run the benchmark.</summary>
/// <param name="instance">The instance.</param>
/// <param name="benchmarkCase">The benchmark.</param>
/// <summary>Fills the properties of the instance of the object used to run the benchmark. Arguments are not supported</summary>
internal static void FillMembers(object instance, BenchmarkCase benchmarkCase)
{
foreach (var parameter in benchmarkCase.Parameters.Items)
{
var flags = BindingFlags.Public;
flags |= parameter.IsStatic ? BindingFlags.Static : BindingFlags.Instance;
FillMember(instance, benchmarkCase, parameter);
}
}

var targetType = benchmarkCase.Descriptor.Type;
var paramProperty = targetType.GetProperty(parameter.Name, flags);
/// <summary>Fills the property of the instance of the object used to run the benchmark. Arguments are not supported</summary>
internal static void FillMember(object instance, BenchmarkCase benchmarkCase, ParameterInstance parameter)
{
var flags = BindingFlags.Public;
flags |= parameter.IsStatic ? BindingFlags.Static : BindingFlags.Instance;

if (paramProperty == null)
{
var paramField = targetType.GetField(parameter.Name, flags);
if (paramField == null)
throw new InvalidOperationException(
$"Type {targetType.FullName}: no property or field {parameter.Name} found.");
var targetType = benchmarkCase.Descriptor.Type;
var paramProperty = targetType.GetProperty(parameter.Name, flags);

var callInstance = paramField.IsStatic ? null : instance;
paramField.SetValue(callInstance, parameter.Value);
}
else
{
var setter = paramProperty.GetSetMethod();
if (setter == null)
throw new InvalidOperationException(
$"Type {targetType.FullName}: no settable property {parameter.Name} found.");
if (paramProperty == null)
{
var paramField = targetType.GetField(parameter.Name, flags);
if (paramField == null)
throw new InvalidOperationException(
$"Type {targetType.FullName}: no property or field {parameter.Name} found.");

var callInstance = setter.IsStatic ? null : instance;
setter.Invoke(callInstance, new[] { parameter.Value });
}
var callInstance = paramField.IsStatic ? null : instance;
paramField.SetValue(callInstance, parameter.Value);
}
else
{
var setter = paramProperty.GetSetMethod();
if (setter == null)
throw new InvalidOperationException(
$"Type {targetType.FullName}: no settable property {parameter.Name} found.");

var callInstance = setter.IsStatic ? null : instance;
setter.Invoke(callInstance, new[] { parameter.Value });
}
}

timcassell marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
11 changes: 5 additions & 6 deletions src/BenchmarkDotNet/Validators/ExecutionValidator.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System;
using System.Collections.Generic;
using BenchmarkDotNet.Running;

namespace BenchmarkDotNet.Validators
{
Expand All @@ -12,20 +11,20 @@ public class ExecutionValidator : ExecutionValidatorBase
private ExecutionValidator(bool failOnError)
: base(failOnError) { }

protected override void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumerable<BenchmarkCase> benchmarks, List<ValidationError> errors)
protected override void ExecuteBenchmarks(IEnumerable<BenchmarkExecutor> executors, List<ValidationError> errors)
{
foreach (var benchmark in benchmarks)
foreach (var executor in executors)
{
try
{
benchmark.Descriptor.WorkloadMethod.Invoke(benchmarkTypeInstance, null);
executor.Invoke();
}
catch (Exception ex)
{
errors.Add(new ValidationError(
TreatsWarningsAsErrors,
$"Failed to execute benchmark '{benchmark.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'",
benchmark));
$"Failed to execute benchmark '{executor.BenchmarkCase.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'",
executor.BenchmarkCase));
}
}
}
Expand Down
Loading