-
-
Notifications
You must be signed in to change notification settings - Fork 810
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
Make interface proxy creation twice as fast #551
Conversation
Creating proxies with DynamicProxy is the single most expensive kind of operation that Moq does. Commit 54e132b changed how proxies for interface types are generated so that `object` methods on interfaces could also be intercepted (and thus set up). Unfortunately, this change also made interface proxy generation twice as costly! This commit restores the previous method for interface proxy creation, and at the same time adds an `InterfaceProxy` class to keep `object` methods interceptable.
a503e7b
to
9c24816
Compare
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.
Looking good, just a few explanatory notes.
// It is public because it needs to be visible to the framework's types in `Reflection.Emit`. | ||
// Not sure what `[assembly: InternalsVisibleTo(<some framework assembly>)]` would have to look like. | ||
[EditorBrowsable(EditorBrowsableState.Never)] | ||
public abstract class InterfaceProxy |
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.
It's a pity this internal type needs to be public
for now. See #98 and https://stackoverflow.com/q/47761193/240733.
{ | ||
// Forward this call to the interceptor, so that `object.Equals` can be set up. | ||
var invocation = new Invocation(equalsMethod, obj); | ||
((IInterceptor)((IMocked)this).Mock).Intercept(invocation); |
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.
When an object
method is invoked on an interface proxy object, DynamicProxy will be bypassed, and we'll end up here, in the base class. The Mock.Intercept(Invocation)
method doesn't care who intercepted a method, so we can just create an Invocation
representing the object
method call, and send it off to be intercepted.
using System.Diagnostics; | ||
using System.Reflection; | ||
|
||
namespace Moq.Internals |
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.
Let's try not to pollute the Moq
namespace with something that should be internal.
Creating proxies with DynamicProxy is the single most expensive kind of operation that Moq does. Commit 54e132b changed how proxies for interface types are generated so that
object
methods on interfaces could also be intercepted (and thus set up). Unfortunately, this change also made interface proxy generation twice as costly!This commit restores the previous method for interface proxy creation, and at the same time adds an
InterfaceProxy
class to keepobject
methods interceptable.This should bring Moq's performance back to, or very close to, what it used to be before version 4.5.16.
/cc #504, #491