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

Fair Return Types #2336

Open
wants to merge 2 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
4 changes: 0 additions & 4 deletions src/BenchmarkDotNet/Code/CodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,12 @@ internal static string Generate(BuildPartition buildPartition)
.Replace("$ID$", buildInfo.Id.ToString())
.Replace("$OperationsPerInvoke$", provider.OperationsPerInvoke)
.Replace("$WorkloadTypeName$", provider.WorkloadTypeName)
.Replace("$WorkloadMethodDelegate$", provider.WorkloadMethodDelegate(passArguments))
.Replace("$WorkloadMethodReturnType$", provider.WorkloadMethodReturnTypeName)
.Replace("$WorkloadMethodReturnTypeModifiers$", provider.WorkloadMethodReturnTypeModifiers)
.Replace("$OverheadMethodReturnTypeName$", provider.OverheadMethodReturnTypeName)
.Replace("$GlobalSetupMethodName$", provider.GlobalSetupMethodName)
.Replace("$GlobalCleanupMethodName$", provider.GlobalCleanupMethodName)
.Replace("$IterationSetupMethodName$", provider.IterationSetupMethodName)
.Replace("$IterationCleanupMethodName$", provider.IterationCleanupMethodName)
.Replace("$OverheadImplementation$", provider.OverheadImplementation)
.Replace("$ConsumeField$", provider.ConsumeField)
.Replace("$JobSetDefinition$", GetJobsSetDefinition(benchmark))
.Replace("$ParamsContent$", GetParamsContent(benchmark))
.Replace("$ArgumentsDefinition$", GetArgumentsDefinition(benchmark))
Expand Down
61 changes: 1 addition & 60 deletions src/BenchmarkDotNet/Code/DeclarationsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,10 @@ internal abstract class DeclarationsProvider

public virtual string WorkloadMethodReturnTypeName => WorkloadMethodReturnType.GetCorrectCSharpTypeName();

public virtual string WorkloadMethodDelegate(string passArguments) => Descriptor.WorkloadMethod.Name;

public virtual string WorkloadMethodReturnTypeModifiers => null;

public virtual string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments})";

public virtual string ConsumeField => null;

protected abstract Type OverheadMethodReturnType { get; }

public string OverheadMethodReturnTypeName => OverheadMethodReturnType.GetCorrectCSharpTypeName();

public abstract string OverheadImplementation { get; }

private string GetMethodName(MethodInfo method)
{
if (method == null)
Expand All @@ -75,62 +65,21 @@ internal class VoidDeclarationsProvider : DeclarationsProvider
public VoidDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }

public override string ReturnsDefinition => "RETURNS_VOID";

protected override Type OverheadMethodReturnType => typeof(void);

public override string OverheadImplementation => string.Empty;
}

internal class NonVoidDeclarationsProvider : DeclarationsProvider
{
public NonVoidDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }

public override string ConsumeField
=> !Consumer.IsConsumable(WorkloadMethodReturnType) && Consumer.HasConsumableField(WorkloadMethodReturnType, out var field)
? $".{field.Name}"
: null;

protected override Type OverheadMethodReturnType
=> Consumer.IsConsumable(WorkloadMethodReturnType)
? WorkloadMethodReturnType
: (Consumer.HasConsumableField(WorkloadMethodReturnType, out var field)
? field.FieldType
: typeof(int)); // we return this simple type because creating bigger ValueType could take longer than benchmarked method itself

public override string OverheadImplementation
{
get
{
string value;
var type = OverheadMethodReturnType;
if (type.GetTypeInfo().IsPrimitive)
value = $"default({type.GetCorrectCSharpTypeName()})";
else if (type.GetTypeInfo().IsClass || type.GetTypeInfo().IsInterface)
value = "null";
else
value = SourceCodeHelper.ToSourceCode(Activator.CreateInstance(type)) + ";";
return $"return {value};";
}
}

public override string ReturnsDefinition
=> Consumer.IsConsumable(WorkloadMethodReturnType) || Consumer.HasConsumableField(WorkloadMethodReturnType, out _)
? "RETURNS_CONSUMABLE"
: "RETURNS_NON_CONSUMABLE_STRUCT";
public override string ReturnsDefinition => "RETURNS_NON_VOID";
}

internal class ByRefDeclarationsProvider : NonVoidDeclarationsProvider
{
public ByRefDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }

protected override Type OverheadMethodReturnType => typeof(IntPtr);

public override string WorkloadMethodReturnTypeName => base.WorkloadMethodReturnTypeName.Replace("&", string.Empty);

public override string ConsumeField => null;

public override string OverheadImplementation => $"return default(System.{nameof(IntPtr)});";

public override string ReturnsDefinition => "RETURNS_BYREF";

public override string WorkloadMethodReturnTypeModifiers => "ref";
Expand All @@ -140,8 +89,6 @@ internal class ByReadOnlyRefDeclarationsProvider : ByRefDeclarationsProvider
{
public ByReadOnlyRefDeclarationsProvider(Descriptor descriptor) : base(descriptor) { }

public override string ReturnsDefinition => "RETURNS_BYREF_READONLY";

public override string WorkloadMethodReturnTypeModifiers => "ref readonly";
}

Expand All @@ -151,9 +98,6 @@ internal class TaskDeclarationsProvider : VoidDeclarationsProvider

// we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way,
// and will eventually throw actual exception, not aggregated one
public override string WorkloadMethodDelegate(string passArguments)
=> $"({passArguments}) => {{ {Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult(); }}";

public override string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult()";

protected override Type WorkloadMethodReturnType => typeof(void);
Expand All @@ -170,9 +114,6 @@ internal class GenericTaskDeclarationsProvider : NonVoidDeclarationsProvider

// we use GetAwaiter().GetResult() because it's fastest way to obtain the result in blocking way,
// and will eventually throw actual exception, not aggregated one
public override string WorkloadMethodDelegate(string passArguments)
=> $"({passArguments}) => {{ return {Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult(); }}";

public override string GetWorkloadMethodCall(string passArguments) => $"{Descriptor.WorkloadMethod.Name}({passArguments}).GetAwaiter().GetResult()";
}
}
33 changes: 0 additions & 33 deletions src/BenchmarkDotNet/Engines/Consumer.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Threading;
using JetBrains.Annotations;
Expand All @@ -11,13 +8,6 @@ namespace BenchmarkDotNet.Engines
{
public class Consumer
{
private static readonly HashSet<Type> SupportedTypes
= new HashSet<Type>(
typeof(Consumer).GetTypeInfo()
.DeclaredFields
.Where(field => !field.IsStatic) // exclude this HashSet itself
.Select(field => field.FieldType));

#pragma warning disable IDE0052 // Remove unread private members
private volatile byte byteHolder;
private volatile sbyte sbyteHolder;
Expand Down Expand Up @@ -153,28 +143,5 @@ public void Consume<T>(in T value)
else
DeadCodeEliminationHelper.KeepAliveWithoutBoxingReadonly(value); // non-primitive and nullable value types
}

internal static bool IsConsumable(Type type)
=> SupportedTypes.Contains(type) || type.GetTypeInfo().IsClass || type.GetTypeInfo().IsInterface;

internal static bool HasConsumableField(Type type, out FieldInfo? consumableField)
{
var typeInfo = type.GetTypeInfo();

if (typeInfo.IsEnum)
{
// Enums are tricky bastards which report "value__" field, which is public for reflection, but inaccessible via C#
consumableField = null;
return false;
}

var publicInstanceFields = typeInfo.DeclaredFields
.Where(field => field.IsPublic && !field.IsStatic)
.ToArray();

consumableField = publicInstanceFields.FirstOrDefault(field => IsConsumable(field.FieldType));

return consumableField != null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,20 @@ public static LocalBuilder DeclareOptionalLocalForReturnDefault(this ILGenerator
: null;
}

public static void EmitSetLocalToDefault(this ILGenerator ilBuilder, LocalBuilder local)
{
var resultType = local.LocalType;
switch (resultType)
{
case Type t when t == typeof(void):
break;
case Type t when t.IsClass || t.IsInterface:
ilBuilder.Emit(OpCodes.Ldnull);
ilBuilder.EmitStloc(local);
break;
case Type t when t.UseInitObjForInitLocal():
EmitInitObj(ilBuilder, resultType, local);
break;
default:
EmitLoadDefaultPrimitive(ilBuilder, resultType);
ilBuilder.EmitStloc(local);
break;
}
}

public static void EmitReturnDefault(this ILGenerator ilBuilder, Type resultType, LocalBuilder optionalLocalForInitobj)
{
switch (resultType)
{
case Type t when t == typeof(void):
break;
case Type t when t.IsPointer: // Type.IsClass returns true for pointers, so we have to check for pointer type first.
/*
IL_0000: ldc.i4.0
IL_0001: conv.u
*/
ilBuilder.Emit(OpCodes.Ldc_I4_0);
ilBuilder.Emit(OpCodes.Conv_U);
break;
case Type t when t.IsClass || t.IsInterface:
ilBuilder.Emit(OpCodes.Ldnull);
break;
Expand Down

This file was deleted.

Loading
Loading