-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
Simplify and optimize interception #523
Conversation
By moving the code that finds a setup for an invocation inside the `lock` of the `InterceptorContext`, we can avoid creating a snapshot copy of all setups for each invocation.
Most methods that get invoked on a delegate won't be event accessors. Therefore, checking for each invocation whether it is such a one should take as little time as possible.
Some well-known methods have predefined behavior (`object` methods, `IMocked.Mock` and `IMocked<T>.Mock`, as well as finalizers). Instead of having three interception strategies for these and going through each sequentially, let's just have one dictionary-based one.
Source/Interceptor.cs
Outdated
HandleTracking.Instance, | ||
InterceptMockPropertyMixin.Instance, | ||
InterceptObjectMethodsMixin.Instance, | ||
HandleWellKnownMethods.Instance, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling finalizers after HandleTracking
might in theory be problematic since the objects related to the one being finalized might already be gone. But then, assuming that the underlying proxy library might intercept a finalizer is just as problematic. (DynamicProxy doesn't intercept finalizers, so all logic in Moq that deals with a finalizer call is really just a backup.)
@@ -261,8 +265,10 @@ public void Verify() | |||
|
|||
private bool TryVerify(out UnmatchedSetups error) | |||
{ | |||
if (!this.Interceptor.TryVerify(out error)) | |||
var uninvokedVerifiableSetups = this.Setups.ToArrayLive(setup => setup.IsVerifiable && !setup.Invoked); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By moving setups from InterceptorContext
into Mock
, verification is no longer spread across three types (Mock
, Interceptor
, InterceptorContext
) but happens completely within Mock
.
@@ -107,6 +101,8 @@ public override Switches Switches | |||
set => this.owner.Switches = value; | |||
} | |||
|
|||
internal override Type TargetType => this.owner.TargetType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is almost identical to MockedType
, except for the overrides in AsInterface
.
This PR achieves two things:
Makes the whole interception mechanism a little simpler and easier to understand.
InterceptorContext
is replaced by dedicated collection properties directly onMock
:Mock.Setups
,Mock.Invocations
, andMock.EventHandlers
. These use dedicated collection types who perform the necessary locking (for thread-safety) internally.Applies several optimizations (such as fewer allocations and collection copying) that speed up mock method interception by approx. 30% (though interception is already pretty fast, so this might not be very noticeable).
(There's possibly room for further speed improvements, esp. regarding the
lock
s inside the collection typesSetupCollection
,InvocationCollection
,EventHandlerCollection
.)