Skip to content

Avoid lambda compilation as much as possible #2957

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

Merged
merged 16 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from 13 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,321 @@
using System;
using System.Linq.Expressions;
using System.Reflection;
using NHibernate.Impl;
using NUnit.Framework;
using Expression = System.Linq.Expressions.Expression;

namespace NHibernate.Test.Criteria.Lambda
{
public static class DateTimeExtensions
{
public static long ToLongDateTime(this DateTime date)
{
return Convert.ToInt64(date.ToString("yyyyMMddHHmmss"));
}
}

[TestFixture]
public class ExpressionProcessorFindValueFixture
{
private static object GetValue<T>(Expression<Func<T>> expression)
{
try
{
return ExpressionProcessor.FindValue(expression.Body);
}
catch (TargetInvocationException e)
{
throw e.InnerException;
}
}

private static int GetIntegerDate()
{
return Convert.ToInt32(DateTime.Now.ToString("yyyyMMdd"));
}

[Test]
public void StaticPropertyInstanceMethodCall()
{
var actual = GetValue(() => DateTime.Now.ToString("yyyyMMdd"));
var expected = DateTime.Now.ToString("yyyyMMdd");

Assert.AreEqual(expected, actual);
}

[Test]
public void StaticPropertyInstanceMultipleMethodCall()
{
var actual = GetValue(() => DateTime.Now.AddDays(365).ToString("yyyyMMdd"));
var expected = DateTime.Now.AddDays(365).ToString("yyyyMMdd");

Assert.AreEqual(expected, actual);
}

[Test]
public void StaticPropertyInstanceMethodCallThenCast()
{
var actual = GetValue(() => Convert.ToInt32(DateTime.Now.AddDays(365).ToString("yyyyMMdd")));
var expected = Convert.ToInt32(DateTime.Now.AddDays(365).ToString("yyyyMMdd"));

Assert.AreEqual(expected, actual);
}

[Test]
public void StaticMethodCall()
{
var actual = GetValue(() => GetIntegerDate());
var expected = GetIntegerDate();

Assert.AreEqual(expected, actual);
}

[Test]
public void ExtensionMethodCall()
{
var date = DateTime.Now;
var actual = GetValue(() => date.ToLongDateTime());
var expected = date.ToLongDateTime();

Assert.AreEqual(expected, actual);
}

[Test]
public void NestedPropertyAccess()
{
var animal = new { Snake = new { Animal = new { Name = "Scorpion" } } };
var actual = GetValue(() => animal.Snake.Animal.Name);
var expected = animal.Snake.Animal.Name;

Assert.AreEqual(expected, actual);
}

[Test]
public void GuidToStringCast()
{
var guid = Guid.NewGuid();
Expression<Func<string>> expression = () => $"{guid}";
var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();
var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void NestedPropertyToIntegerCast()
{
var animal = new { Snake = new { Animal = new { Weight = 9.89 } } };
Expression<Func<int>> expression = () => (int) animal.Snake.Animal.Weight;
var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();
var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void NestedPropertyToIntegerConvert()
{
var animal = new { Snake = new { Animal = new { Weight = 9.89 } } };
Expression<Func<int>> expression = () => Convert.ToInt32(animal.Snake.Animal.Weight);
var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();
var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void NullToIntegerCastFails()
{
object value = null;
Expression<Func<int>> expression = () => (int) value;

Assert.Throws<NullReferenceException>(() => GetValue(expression));

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

Assert.Throws<NullReferenceException>(() => ((dynamic) lambdaExpression.DynamicInvoke()).Invoke());
}

[Test]
public void NullableIntegerToIntegerCastFails()
{
int? value = null;
Expression<Func<int>> expression = () => (int) value;

Assert.Throws<InvalidOperationException>(() => GetValue(expression));

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

Assert.Throws<InvalidOperationException>(() => ((dynamic) lambdaExpression.DynamicInvoke()).Invoke());
}

[Test]
public void NullableIntegerToIntegerCast()
{
int? value = 1;
Expression<Func<int>> expression = () => (int) value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void NullableIntegerToNullableLongImplicitCast()
{
int? value = 1;
Expression<Func<long?>> expression = () => value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void IntegerToIntegerCast()
{
int value = 1;
Expression<Func<int>> expression = () => (int) value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void IntegerToNullableIntegerImplicitCast()
{
int value = 12345;
Expression<Func<int?>> expression = () => value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void IntegerToNullableLongImplicitCast()
{
int value = 12345;
Expression<Func<long?>> expression = () => value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void IntegerToNullableDecimalImplicitCast()
{
int value = 12345;
Expression<Func<decimal?>> expression = () => value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);
}

[Test]
public void Int16ToIntegerImplicitCast()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would fall into 'lambda compile' path. Do we want to fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to fix it bu convert is to much tricky dou you have any idea to fix it ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in #2957 (comment)

Let's start from something simple but safe and make it more complex and sophisticated in other PRs. Unless you already have in mind some simple fix that you want to stick in this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you already have in mind some simple fix

It is a variation of ConvertType in the expression processor. Convert for primitive types (IsPrimitive):

The primitive types are Boolean, Byte, SByte, Int16, UInt16, Int32, UInt32, Int64, UInt64, IntPtr, UIntPtr, Char, Double, and Single.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a variation of ConvertType

I would do it in another PR. It doesn't look as straightforward fix to me. For instance Boolean type and value overflows might behave differently in runtime vs ChangeType.

Copy link
Contributor Author

@gokhanabatay gokhanabatay Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @bahusoid, in my test I was stuck with NestedPropertyToIntegerCast and StringToIntegerCastFails failed with Convert.ChangeType
Such as
Convert.ChangeType(9.89m, typeof(int)) = 10, (int)9.89m = 9
Convert.ChangeType(true, typeof(int)) = 1, (int)true = fails

I also tried with T Cast<T>(object value) => return (T)value; store static instance of CastMethodInfo and make generic method before invoke but this was also failed. Which may run for primitive cases, for performance i am not sure of it? @hazzik

{
short value = 12345;
Expression<Func<int>> expression = () => value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);

}

[Test]
public void NullableDecimalToDecimalCast()
{
decimal? value = 9.89m;
Expression<Func<decimal>> expression = () => (decimal) value;

var actual = GetValue(expression);

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();
var expected = ((dynamic) lambdaExpression.DynamicInvoke()).Invoke();

Assert.AreEqual(expected, actual);

}

[Test]
public void StringToIntegerCastFails()
{
object value = "Abatay";
Expression<Func<int>> expression = () => (int) value;

Assert.Throws<InvalidCastException>(() => GetValue(expression));

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

Assert.Throws<InvalidCastException>(() => ((dynamic) lambdaExpression.DynamicInvoke()).Invoke());
}

[Test]
public void BooleanToCharCastFails()
{
object isTrue = true;
Expression<Func<char>> expression = () => (char) isTrue;

Assert.Throws<InvalidCastException>(() => GetValue(expression));

//Check with expression compile and invoke
var lambdaExpression = Expression.Lambda(expression).Compile();

Assert.Throws<InvalidCastException>(() => ((dynamic) lambdaExpression.DynamicInvoke()).Invoke());
}
}
}
53 changes: 38 additions & 15 deletions src/NHibernate/Impl/ExpressionProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -248,32 +248,55 @@ private static ICriterion Le(ProjectionInfo property, object value)
}

/// <summary>
/// Invoke the expression to extract its runtime value
/// Walk or Invoke expression to extract its runtime value
/// </summary>
public static object FindValue(Expression expression)
{
if (expression.NodeType == ExpressionType.Constant)
return ((ConstantExpression) expression).Value;
object findValue(Expression e) => e != null ? FindValue(e) : null;

if (expression.NodeType == ExpressionType.MemberAccess)
switch (expression.NodeType)
{
var memberAccess = (MemberExpression) expression;
if (memberAccess.Expression == null || memberAccess.Expression.NodeType == ExpressionType.Constant)
{
var constantValue = ((ConstantExpression) memberAccess.Expression)?.Value;
var member = memberAccess.Member;
switch (member.MemberType)
case ExpressionType.Constant:
var constantExpression = (ConstantExpression) expression;
return constantExpression.Value;
case ExpressionType.MemberAccess:
var memberExpression = (MemberExpression) expression;
switch (memberExpression.Member.MemberType)
{
case MemberTypes.Field:
return ((FieldInfo) member).GetValue(constantValue);
return ((FieldInfo) memberExpression.Member).GetValue(findValue(memberExpression.Expression));
case MemberTypes.Property:
return ((PropertyInfo) member).GetValue(constantValue);
return ((PropertyInfo) memberExpression.Member).GetValue(findValue(memberExpression.Expression));
}
}
break;
case ExpressionType.Call:
var methodCallExpression = (MethodCallExpression) expression;
var args = methodCallExpression.Arguments.ToArray(arg => FindValue(arg));
var callingObject = findValue(methodCallExpression.Object);
return methodCallExpression.Method.Invoke(callingObject, args);
case ExpressionType.Convert:
var unaryExpression = (UnaryExpression) expression;

if (unaryExpression.Method != null)
return unaryExpression.Method.Invoke(null, new[] { FindValue(unaryExpression.Operand) });

var toType = unaryExpression.Type;
var fromType = unaryExpression.Operand.Type;
if (toType == typeof(object) || toType == fromType || Nullable.GetUnderlyingType(toType) == fromType)
return FindValue(unaryExpression.Operand);

if (toType == Nullable.GetUnderlyingType(fromType))
{
var val = FindValue(unaryExpression.Operand);
if (val != null)
return val;
}

break;
}

var valueExpression = Expression.Lambda(expression).Compile();
object value = valueExpression.DynamicInvoke();
var lambdaExpression = Expression.Lambda(expression).Compile(true);
var value = lambdaExpression.DynamicInvoke();
return value;
}

Expand Down
Loading