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.

Sign up for GitHub

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

Jump to bottom

EquivalencyValidator performance fixes #935

Merged
merged 2 commits into from Nov 20, 2018
Merged

EquivalencyValidator performance fixes #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8c7c7a
EquivalencyValidator performance fixes
pentp Oct 2, 2018
bf43168
Address PR feedback
pentp Nov 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions Src/FluentAssertions/Common/ExpressionExtensions.cs
View file
Open in desktop
Expand Up @@ -158,10 +158,5 @@ internal static class ExpressionExtensions

return new MemberPath(declaringType, segmentPath.Replace(".[", "["));
}

internal static string GetMethodName(Expression<Action> action)
{
return ((MethodCallExpression)action.Body).Method.Name;
}
}
}
41 changes: 13 additions & 28 deletions Src/FluentAssertions/Equivalency/GenericDictionaryEquivalencyStep.cs
View file
Open in desktop
@@ -1,9 +1,8 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using FluentAssertions.Common;
using FluentAssertions.Execution;

namespace FluentAssertions.Equivalency
Expand All @@ -14,6 +13,12 @@ namespace FluentAssertions.Equivalency
/// </remarks>
public class GenericDictionaryEquivalencyStep : IEquivalencyStep
{
private static readonly MethodInfo AssertSameLengthMethod = new Func<IDictionary<object, object>, IDictionary<object, object>, bool>
(AssertSameLength).GetMethodInfo().GetGenericMethodDefinition();

private static readonly MethodInfo AssertDictionaryEquivalenceMethod = new Action<EquivalencyValidationContext, IEquivalencyValidator, IEquivalencyAssertionOptions, IDictionary<object, object>, IDictionary<object, object>>
(AssertDictionaryEquivalence).GetMethodInfo().GetGenericMethodDefinition();

public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAssertionOptions config)
{
Type expectationType = config.GetExpectationType(context);
Expand Down Expand Up @@ -135,22 +140,17 @@ private static Type GetDictionaryKeyType(Type expectedType)

private static bool AssertSameLength(object subject, Type expectationType, object expectation)
{
string methodName =
ExpressionExtensions.GetMethodName(() => AssertSameLength<object, object, object, object>(null, null));
if(subject is ICollection subjectCollection
&& expectation is ICollection expectationCollection
&& subjectCollection.Count == expectationCollection.Count)
return true;

Type subjectType = subject.GetType();
Type[] subjectTypeArguments = GetDictionaryTypeArguments(subjectType);
Type[] expectationTypeArguments = GetDictionaryTypeArguments(expectationType);
Type[] typeArguments = subjectTypeArguments.Concat(expectationTypeArguments).ToArray();

MethodCallExpression assertSameLength = Expression.Call(
typeof(GenericDictionaryEquivalencyStep),
methodName,
typeArguments,
Expression.Constant(subject, GetIDictionaryInterface(subjectType)),
Expression.Constant(expectation, GetIDictionaryInterface(expectationType)));

return (bool)Expression.Lambda(assertSameLength).Compile().DynamicInvoke();
return (bool)AssertSameLengthMethod.MakeGenericMethod(typeArguments).Invoke(null, new[] { subject, expectation });
}

private static Type[] GetDictionaryTypeArguments(Type type)
Expand Down Expand Up @@ -233,27 +233,12 @@ private static Type GetIDictionaryInterface(Type expectedType)
IEquivalencyValidator parent, IEquivalencyAssertionOptions config)
{
Type expectationType = config.GetExpectationType(context);

string methodName =
ExpressionExtensions.GetMethodName(
() => AssertDictionaryEquivalence<object, object, object, object>(null, null, null, null, null));

Type subjectType = context.Subject.GetType();
Type[] subjectTypeArguments = GetDictionaryTypeArguments(subjectType);
Type[] expectationTypeArguments = GetDictionaryTypeArguments(expectationType);
Type[] typeArguments = subjectTypeArguments.Concat(expectationTypeArguments).ToArray();

MethodCallExpression assertDictionaryEquivalence = Expression.Call(
typeof(GenericDictionaryEquivalencyStep),
methodName,
typeArguments,
Expression.Constant(context),
Expression.Constant(parent),
Expression.Constant(config),
Expression.Constant(context.Subject, GetIDictionaryInterface(subjectType)),
Expression.Constant(context.Expectation, GetIDictionaryInterface(expectationType)));

Expression.Lambda(assertDictionaryEquivalence).Compile().DynamicInvoke();
AssertDictionaryEquivalenceMethod.MakeGenericMethod(typeArguments).Invoke(null, new[] { context, parent, config, context.Subject, context.Expectation });
}

private static void AssertDictionaryEquivalence<TSubjectKey, TSubjectValue, TExpectedKey, TExpectedValue>(
Expand Down
40 changes: 13 additions & 27 deletions Src/FluentAssertions/Equivalency/GenericEnumerableEquivalencyStep.cs
View file
Open in desktop
Expand Up @@ -2,7 +2,6 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using FluentAssertions.Common;
using FluentAssertions.Execution;
Expand All @@ -11,6 +10,9 @@ namespace FluentAssertions.Equivalency
{
public class GenericEnumerableEquivalencyStep : IEquivalencyStep
{
private static readonly MethodInfo HandleMethod = new Action<EnumerableEquivalencyValidator, object[], IEnumerable<object>>
(HandleImpl).GetMethodInfo().GetGenericMethodDefinition();

/// <summary>
/// Gets a value indicating whether this step can handle the verificationScope subject and/or expectation.
/// </summary>
Expand All @@ -36,14 +38,13 @@ public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAsserti
{
Type expectedType = config.GetExpectationType(context);

var interfaceTypes = GetIEnumerableInterfaces(expectedType)
.Select(type => "IEnumerable<" + type.GetGenericArguments().Single() + ">")
.ToList();
var interfaceTypes = GetIEnumerableInterfaces(expectedType);

AssertionScope.Current
.ForCondition(interfaceTypes.Count == 1)
.FailWith("{context:Expectation} implements {0}, so cannot determine which one " +
"to use for asserting the equivalency of the collection. ", interfaceTypes);
.ForCondition(interfaceTypes.Length == 1)
.FailWith(() => new FailReason("{context:Expectation} implements {0}, so cannot determine which one " +
"to use for asserting the equivalency of the collection. ",
interfaceTypes.Select(type => "IEnumerable<" + type.GetGenericArguments().Single() + ">")));

if (AssertSubjectIsCollection(context.Expectation, context.Subject))
{
Expand All @@ -55,20 +56,11 @@ public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAsserti

Type typeOfEnumeration = GetTypeOfEnumeration(expectedType);

MethodCallExpression expectationAsArray = ToArray(context.Expectation, typeOfEnumeration);
ConstantExpression subjectAsArray =
Expression.Constant(EnumerableEquivalencyStep.ToArray(context.Subject));

MethodCallExpression executeExpression = Expression.Call(
Expression.Constant(validator),
ExpressionExtensions.GetMethodName(() => validator.Execute<object>(null, null)),
new[] {typeOfEnumeration},
subjectAsArray,
expectationAsArray);
var subjectAsArray = EnumerableEquivalencyStep.ToArray(context.Subject);

try
{
Expression.Lambda(executeExpression).Compile().DynamicInvoke();
HandleMethod.MakeGenericMethod(typeOfEnumeration).Invoke(null, new[] { validator, subjectAsArray, context.Expectation });
}
catch (TargetInvocationException e)
{
Expand All @@ -79,6 +71,9 @@ public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAsserti
return true;
}

private static void HandleImpl<T>(EnumerableEquivalencyValidator validator, object[] subject, IEnumerable<T> expectation)
=> validator.Execute(subject, expectation?.ToArray());

private static bool AssertSubjectIsCollection(object expectation, object subject)
{
bool conditionMet = AssertionScope.Current
Expand Down Expand Up @@ -120,14 +115,5 @@ private static Type GetTypeOfEnumeration(Type enumerableType)

return interfaceType.GetGenericArguments().Single();
}

private static MethodCallExpression ToArray(object value, Type typeOfEnumeration)
{
return Expression.Call(
typeof(Enumerable),
"ToArray",
new[] {typeOfEnumeration},
Expression.Constant(value, typeof(IEnumerable<>).MakeGenericType(typeOfEnumeration)));
}
}
}
24 changes: 11 additions & 13 deletions Src/FluentAssertions/Equivalency/ObjectReference.cs
View file
Open in desktop
Expand Up @@ -10,18 +10,14 @@ namespace FluentAssertions.Equivalency
internal class ObjectReference
{
private readonly object @object;
private readonly string[] path;
private readonly string path;
private readonly bool? isComplexType;
private string[] pathElements;

public ObjectReference(object @object, string path, bool? isComplexType = null)
{
this.@object = @object;

this.path = path
.ToLower()
.Replace("][", "].[")
.Split(new[] { '.' }, StringSplitOptions.RemoveEmptyEntries);

this.path = path;
this.isComplexType = isComplexType;
}

Expand All @@ -42,9 +38,14 @@ public override bool Equals(object obj)
return ReferenceEquals(@object, other.@object) && IsParentOf(other);
}

private string[] GetPathElements() => pathElements
?? (pathElements = path.ToLowerInvariant().Replace("][", "].[").Split(new[] { '.' }, StringSplitOptions.RemoveEmptyEntries));

private bool IsParentOf(ObjectReference other)
{
return (other.path.Length > path.Length) && other.path.Take(path.Length).SequenceEqual(path);
string[] path = GetPathElements();
string[] otherPath = other.GetPathElements();
return (otherPath.Length > path.Length) && otherPath.Take(path.Length).SequenceEqual(path);
}

/// <summary>
Expand All @@ -56,15 +57,12 @@ private bool IsParentOf(ObjectReference other)
/// <filterpriority>2</filterpriority>
public override int GetHashCode()
{
unchecked
{
return (@object.GetHashCode() * 397) ^ path.GetHashCode();
}
return System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(@object);
Copy link
Member

Choose a reason for hiding this comment

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

❓ I guess you saw that getting HashCode of the old path array, didn't make much sense.
Can you comment on why you chose RuntimeHelpers.GetHashCode over Object.GetHashCode()?

Copy link
Contributor Author

@pentp pentp Nov 9, 2018 •
edited

Choose a reason for hiding this comment

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

Because we want to compare/hash the object reference not value. RuntimeHelpers.GetHashCode returns the object identity hash even if it overrides Object.GetHashCode.
I'm not sure if this is even used anywhere, it looks like ObjectReference is only used in lists.

Copy link
Member

Choose a reason for hiding this comment

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

I tried a dig a bit in the git history to get some context:

}

public override string ToString()
{
return $"{{\"{string.Join(".", path)}\", {@object}}}";
return $"{{\"{path}\", {@object}}}";
}

public bool IsComplexType => isComplexType ?? (!(@object is null) && !@object.GetType().OverridesEquals());
Expand Down