Reduce use of reflection in SetupAllProperties
#549
Conversation
| foreach (var property in properties) | ||
| { | ||
| var expression = GetPropertyExpression(mockType, property); | ||
| object initialValue = GetInitialValue(mock, mockedTypesStack, property); | ||
| object value = GetInitialValue(mock, mockedTypesStack, property); |
There was a problem hiding this comment.
Renamed because this variable no longer just holds the initial value; it acts as the backing field for the set up property. It gets captured by both setups' getter / setter lambdas.
| /// Only use this constructor when you know that the specified <paramref name="method"/> has no `out` parameters, | ||
| /// and when you want to avoid the <see cref="MatcherFactory"/>-related overhead of the other constructor overload. | ||
| /// </remarks> | ||
| public MethodCall(Mock mock, Condition condition, Expression originalExpression, MethodInfo method, IMatcher[] argumentMatchers) |
There was a problem hiding this comment.
For the type of setups we want to install (i. e. setups that implement property getters and setters), we know in advance what kinds of matchers we'll need. The existing constructor would however force us to build an It.IsAny<T>() expression, which would then be put through the (slow!) MatcherFactory.CreateMatcher factory method. This new constructor allows us to skip these two unnecessary steps.
| if (mocked != null) | ||
| { | ||
| SetupAllProperties(mocked.Mock, mockedTypesStack); | ||
| } | ||
|
|
||
| mock.Setups.Add(new PropertyGetterMethodCall(mock, expression, property.GetGetMethod(true), () => value)); |
There was a problem hiding this comment.
The above LINQ query only returns properties that are readable, so we don't need to check property.CanRead.
|
Glad to see this! Perf. should be comparable with v5, see the just-added PropertyBehavior :) It's added wholesale to every loose mock by default so |
|
@kzu - should something similar be done in Moq 4? Auto setting up property behavior for get-settable properties of loose mocks seems entirely feasible; we could modify the interception pipeline such that when a property is queried or set for the first time, a property setup is created on the fly. (This might be perceived as a breaking change, though.) |
|
Btw., regarding performance: Moq 4 performance is now back to what it was around version 4.2 (despite being more thread-safe and feature complete). I am expecting that it will get even faster once the remaining filed bugs are removed--essentially because fixing them will entail reducing expression tree compilation and doing plain expression tree rewriting instead, which is orders of magnitude faster. In particular, perf will greatly benefit on x64. I've prototyped the necessary changes, but it'll be quite difficult to pull this off without causing any minor breaking changes. |
Excessive reflection in
SetupAllPropertiescontributes toMock.Of<T>'s bad performance (#547).This PR reduces use of reflection in this method by adding instances of specialized
MethodCalltypes directly tomock.Setups, instead of installing setups by making calls tomock.SetupProperty(m => m.Property, initialValue)ormock.SetupGet(m => m.Property).Returns(value)through reflection.