Skip to content

Commit

Permalink
added: some initial improvements for #196: condition reduction, false…
Browse files Browse the repository at this point in the history
… and true constants singletons; TBD: look to try catch

upped language to 7.3
  • Loading branch information
dadhi committed May 30, 2019
1 parent 4e10b32 commit c164134
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 31 deletions.
2 changes: 2 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<!--Set to true to minimize number of target frameworks, to fasten the build :-) -->
<DevMode>true</DevMode>

<LangVersion>7.3</LangVersion>

<Authors>Maksim Volkau</Authors>
<Copyright>Copyright © 2016-2019 Maksim Volkau</Copyright>
<NeutralLanguage>en-US</NeutralLanguage>
Expand Down
15 changes: 11 additions & 4 deletions src/FastExpressionCompiler.LightExpression/Expression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,18 @@ public static ParameterExpression Parameter(Type type, string name = null) =>

public static ParameterExpression Variable(Type type, string name = null) => Parameter(type, name);

public static ConstantExpression Constant(object value, Type type = null) =>
value == null && type == null ? _nullExprInfo
: new ConstantExpression(value, type ?? value.GetType());
public static ConstantExpression Constant(object value, Type type = null)
{
if (value is bool b)
return b ? _trueExpr : _falseExpr;
if (type == null)
return value != null ? new ConstantExpression(value, value.GetType()) : _nullExpr;
return new ConstantExpression(value, type);
}

private static readonly ConstantExpression _nullExprInfo = new ConstantExpression(null, typeof(object));
private static readonly ConstantExpression _nullExpr = new ConstantExpression(null, typeof(object));
private static readonly ConstantExpression _falseExpr = new ConstantExpression(false, typeof(bool));
private static readonly ConstantExpression _trueExpr = new ConstantExpression(true, typeof(bool));

public static NewExpression New(Type type) =>
new NewExpression(type.GetTypeInfo().DeclaredConstructors.First(x => x.GetParameters().Length == 0), Tools.Empty<Expression>());
Expand Down
64 changes: 48 additions & 16 deletions src/FastExpressionCompiler/FastExpressionCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3282,6 +3282,7 @@ var methodName
return false;

EmitMethodCall(il, _objectEqualsMethod);

if (expressionType == ExpressionType.NotEqual) // invert result for not equal
{
il.Emit(OpCodes.Ldc_I4_0);
Expand Down Expand Up @@ -3621,39 +3622,29 @@ private static bool TryEmitLogicalOperator(BinaryExpression expr,
private static bool TryEmitConditional(ConditionalExpression expr,
IReadOnlyList<ParameterExpression> paramExprs, ILGenerator il, ref ClosureInfo closure, ParentFlags parent)
{
var testExpr = expr.Test;
var usedInverted = false;
var testExpr = TryReduceCondition(expr.Test);
var comparedWithNull = false;

// optimization: special handling of comparing with null
if (testExpr is BinaryExpression b &&
((testExpr.NodeType == ExpressionType.Equal || testExpr.NodeType == ExpressionType.NotEqual) &&
!(b.Left.Type.IsNullable() || b.Right.Type.IsNullable()) &&
b.Right is ConstantExpression r && r.Value == null
? TryEmit(b.Left, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult)
: b.Left is ConstantExpression l && l.Value == null &&
TryEmit(b.Right, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult)))
{
usedInverted = true;
}
if (testExpr is BinaryExpression b && TryEmitComparisonWithNull(b, paramExprs, il, ref closure, parent))
comparedWithNull = true;
else if (!TryEmit(testExpr, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult))
return false;

var labelIfFalse = il.DefineLabel();
il.Emit(usedInverted && testExpr.NodeType == ExpressionType.Equal ? OpCodes.Brtrue : OpCodes.Brfalse, labelIfFalse);
il.Emit(comparedWithNull && testExpr.NodeType == ExpressionType.Equal ? OpCodes.Brtrue : OpCodes.Brfalse, labelIfFalse);

var ifTrueExpr = expr.IfTrue;
if (!TryEmit(ifTrueExpr, paramExprs, il, ref closure, parent & ParentFlags.IgnoreResult))
return false;

var ifFalseExpr = expr.IfFalse;
if ((ifFalseExpr.NodeType == ExpressionType.Default) && (ifFalseExpr.Type == typeof(void)))
if (ifFalseExpr.NodeType == ExpressionType.Default && ifFalseExpr.Type == typeof(void))
{
il.MarkLabel(labelIfFalse);
return true;
}

var labelDone = il.DefineLabel();

il.Emit(OpCodes.Br, labelDone);

il.MarkLabel(labelIfFalse);
Expand All @@ -3664,6 +3655,47 @@ private static bool TryEmitConditional(ConditionalExpression expr,
return true;
}

private static Expression TryReduceCondition(Expression testExpr)
{
if (testExpr is BinaryExpression b)
{
if (b.NodeType == ExpressionType.OrElse)
{
if (b.Left is ConstantExpression l && l.Value is bool lb)
return lb ? b.Left : TryReduceCondition(b.Right);

if (b.Right is ConstantExpression r && r.Value is bool rb && rb == false)
return TryReduceCondition(b.Left);
}
else if (b.NodeType == ExpressionType.AndAlso)
{
if (b.Left is ConstantExpression l && l.Value is bool lb)
return !lb ? b.Left : TryReduceCondition(b.Right);

if (b.Right is ConstantExpression r && r.Value is bool rb && rb == true)
return TryReduceCondition(b.Left);
}
}

return testExpr;
}

private static bool TryEmitComparisonWithNull(BinaryExpression b,
IReadOnlyList<ParameterExpression> paramExprs, ILGenerator il, ref ClosureInfo closure, ParentFlags parent)
{
if (b.NodeType != ExpressionType.Equal && b.NodeType != ExpressionType.NotEqual &&
(b.Left.Type.IsNullable() || b.Right.Type.IsNullable()))
return false;

if (b.Right is ConstantExpression r && r.Value == null)
return TryEmit(b.Left, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult);

if (b.Left is ConstantExpression l && l.Value == null)
return TryEmit(b.Right, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult);

return false;
}

private static bool EmitMethodCall(ILGenerator il, MethodInfo method, ParentFlags parent = ParentFlags.Empty)
{
if (method == null)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks Condition="'$(DevMode)' != 'true'">net46;netcoreapp2.0</TargetFrameworks>
<TargetFrameworks Condition="'$(DevMode)' == 'true'">netcoreapp2.0</TargetFrameworks>
<LangVersion>7.1</LangVersion>
<TargetFrameworks Condition="'$(DevMode)' == 'true'">netcoreapp2.0</TargetFrameworks>
<!--<TargetFramework>net46</TargetFramework>--><!--net46 is for ILVisualizer to work-->
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
<NoWarn>1701;1702;1705;659</NoWarn>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="AutoMapper" Version="5.1.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
<PackageReference Include="NUnit" Version="3.11.0" />
<PackageReference Include="NUnit3TestAdapter" Version="3.10.0" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@


using static System.Linq.Expressions.Expression;
#if !LIGHT_EXPRESSION
using System.Linq;
using System.Reflection;
using System;
using System.Linq.Expressions;
using AutoMapper;
using NUnit.Framework;

namespace FastExpressionCompiler.IssueTests
{
[TestFixture]
public class Issue196_AutoMapper_tests_are_failing_when_using_FEC
{
public class FastExpressionCompilerBug
{
[Test]
public void ShouldWork()
{
var config = new MapperConfiguration(cfg => cfg.CreateMap<Source, Dest>());
var mapper = config.CreateMapper();
var expression = mapper.ConfigurationProvider.ExpressionBuilder.CreateMapExpression(typeof(Source), typeof(Dest));
Assert.IsNotNull(((LambdaExpression)expression).CompileFast(true));

var source = new Source { Value = 5 };
var dest = mapper.Map<Dest>(source);

Assert.AreEqual(source.Value, dest.Value);
}

public class Source
{
public int Value { get; set; }
}

public class Dest
{
public int Value { get; set; }
}

[Test]
public void Comparison_with_null_should_produce_optimal_Brtrue_or_Brfalse_opcodes()
{
var sourceParam = Parameter(typeof(Source), "source");
var body = Condition(
Equal(sourceParam, Constant(null, typeof(Source))),
Constant(null, typeof(Dest)),
MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()),
Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(),
Property(sourceParam, nameof(Source.Value)))));

var expr = Lambda<Func<Source, Dest>>(body, sourceParam);

//var fs = expr.Compile();
var ff = expr.CompileFast(true);

var dest = ff(new Source {Value = 42});
Assert.AreEqual(42, dest.Value);
}

[Test]
public void Logical_OrElse_should_be_reduced_if_one_of_operands_is_known_boolean_value()
{
var sourceParam = Parameter(typeof(Source), "source");
var body = Condition(
OrElse(Equal(sourceParam, Constant(null, typeof(Source))), Constant(false)),
Constant(null, typeof(Dest)),
MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()),
Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(),
Property(sourceParam, nameof(Source.Value)))));

var expr = Lambda<Func<Source, Dest>>(body, sourceParam);

var fs = expr.Compile();
var ff = expr.CompileFast(true);

var dest = ff(new Source { Value = 42 });
Assert.AreEqual(42, dest.Value);
}

[Test]
public void Coalesce_should_produce_optimal_opcodes()
{
var sourceParam = Parameter(typeof(Source), "source");
var body = Condition(
Equal(sourceParam, Constant(null)),
Constant(null, typeof(Dest)),
Coalesce(
Constant(new Dest { Value = 13 }),
MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()),
Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(),
Property(sourceParam, nameof(Source.Value))))));

var expr = Lambda<Func<Source, Dest>>(body, sourceParam);

//var fs = expr.Compile();
var ff = expr.CompileFast(true);

var dest = ff(new Source { Value = 42 });
Assert.AreEqual(13, dest.Value);
}
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
<PropertyGroup>
<TargetFrameworks Condition="'$(DevMode)' != 'true'">net46;netcoreapp2.0</TargetFrameworks>
<TargetFrameworks Condition="'$(DevMode)' == 'true'">netcoreapp2.0</TargetFrameworks>
<LangVersion>7.1</LangVersion>
<DefineConstants>LIGHT_EXPRESSION</DefineConstants>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ public class ObjectMethodExecutorCompiledFastClosure

private ObjectMethodExecutorCompiledFastClosure(MethodInfo methodInfo, TypeInfo targetTypeInfo, object[] parameterDefaultValues)
{
if (methodInfo == null)
{
throw new ArgumentNullException(nameof(methodInfo));
}

MethodInfo = methodInfo;
MethodInfo = methodInfo ?? throw new ArgumentNullException(nameof(methodInfo));
MethodParameters = methodInfo.GetParameters();
TargetTypeInfo = targetTypeInfo;
MethodReturnType = methodInfo.ReturnType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
<PropertyGroup>
<TargetFrameworks Condition="'$(DevMode)' != 'true'">net46;netcoreapp2.0</TargetFrameworks>
<TargetFrameworks Condition="'$(DevMode)' == 'true'">netcoreapp2.0</TargetFrameworks>
<LangVersion>7.1</LangVersion>
<DefineConstants>LIGHT_EXPRESSION</DefineConstants>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
<PropertyGroup>
<TargetFrameworks Condition="'$(DevMode)' != 'true'">net46;netcoreapp2.0</TargetFrameworks>
<TargetFrameworks Condition="'$(DevMode)' == 'true'">netcoreapp2.0</TargetFrameworks>
<LangVersion>7.1</LangVersion>
</PropertyGroup>

<ItemGroup>
Expand Down

0 comments on commit c164134

Please sign in to comment.