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.

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

Slower performance after upgrading from 4.2.1510 to 4.14.7 #1083

Closed
vruss opened this issue Oct 27, 2020 · 5 comments
Closed

Slower performance after upgrading from 4.2.1510 to 4.14.7 #1083

vruss opened this issue Oct 27, 2020 · 5 comments

Comments

@vruss
Copy link

vruss commented Oct 27, 2020

I'm trying to troubleshoot why the test duration has increased by several seconds on a lot of tests (which amounts to double-digit minutes total).

Synthetic tests show that the newer Moq version is a lot faster with both new Mock and Mock.Of. But real testcases are significantly slower.

I think the issue is somewhat related to this Castle issue. As we've also updated Castle from 4.3.3 to 4.4.1, I'm not sure if the issue is with Castle or Moq.

Are there any known performance issues where mocked interfaces (and classes) with A LOT of properties and methods with multiple layers of inheritance with even more properties have become slower in newer updates?

I'm sorry I can't provide a repro as it's production code and I've yet to successfully reproduce it myself in a contained unit.

@stakx
Copy link
Contributor

stakx commented Oct 27, 2020

There's quite frankly nothing we can do to help you without some code we can run and benchmark, so I am going to have to close this issue... sorry. A few remarks though:

  • Is the Moq update the only thing you did? Or did you perhaps update the .NET Framework or switch platform? That could have a massive influence on runtime performance; .NET Core 3 is much, much faster than the .NET Framework, for example, so you may want to consider switching.

  • To the best of my knowledge, DynamicProxy 4.x versions have been fairly stable. I wouldn't expect huge performance differences there. I don't know about earlier versions (3.x).

  • That being said, Moq performance is mostly dictated by the time spent in DynamicProxy, which spends most of its time in System.Reflection[.Emit]. For that reason, it pays off to be aware of what types you're mocking. The more different types you mock, and the more members those types have, the worse performance you'll get. Favor small role interfaces over complex / monolithlic base classes in your tests.

  • Where possible, don't reuse mocks between tests. Let each tests have its separate mock(s), and set up only what's necessary. Mocks with few setups should tend to be faster than mocks with a ton of setups to satisfy the needs of many diverse tests. Separate mocks are also beneficial because because mocks use locking for thread-safety. If you share the same mocks between tests, and you run tests in parallel, the test runner threads might end up having to wait their turn.

  • Moq versions 4.8+ should have runtime performance characteristics similar to early 4.0-4.2 versions. They're a little slower (due to many new features I'd guess) but not by an order of magnitude or more (last time I checked, anyway). 4.12 may very well be the fastest; I've focused more on consistency and correctness of features in newer versions and that may have somewhat brought down speed once again.

@stakx stakx closed this as completed Oct 27, 2020
@stakx
Copy link
Contributor

stakx commented Oct 27, 2020

P.S. it's good you already found the Castle.Core issue. My advice there re: proxying huge types is equally valid for Moq.

Also, if you do manage to prepare a test project, please post it here and we'll reopen this issue!

@Znurre
Copy link

Znurre commented Oct 29, 2020

Thank you for the great replies.

While not a direct lead on the performance regression itself, we have had very good experience in our own code base (which is really heavy on Emit usage) by changing from System.Emit to Mono.Cecil.
Would you be interested in accepting a contribution upstream for an alternative ProxyFactory implementation utilizing Mono.Cecil instead of Castle if we can prove a large performance benefit in our scenario? Or alternatively, do you think such a contribution would be better directed towards an alternative backend for Castle.Core?

If you are interested in accepting an alternative backend for Moq, how do you suggest we go about implementing the logic for switching between the two backends for the end user?

@stakx
Copy link
Contributor

stakx commented Oct 29, 2020

@Znurre, the default proxy factory should remain DynamicProxy, however adding the ability to opt-in to using a different one is a possibility.

Perhaps the easiest place to add this is ProxyFactory (which isn't publicly exposed at this time), and its static Instance property (which is currently read-only).

At some point in the past I have wanted to expose ProxyFactory, however I'm still not quite satisfied with its public interface. One would actually have to implement a few more proxy factories to see whether its interface sufficiently covers their varying abilities and constraints; and then possibly adapt it, and all of Moq's internals, accordingly.

Now, while the above may be the easiest way to allow you to swap out DynamicProxy, it isn't necessarily the best.

It might actually be neater to expose a ProxyFactory property in MockRepository (and optionally, in Mock), as is done for other things like DefaultValueProvider. I've toyed with that approach before.

This also causes a large amount of necessary refactoring work because you have to query for these instance properties instead of a well-known global static property in a variety of places. Which is why I haven't done it yet. No point in complicating code for a feature that only very few users are going to use.

In my personal opinion, this doesn't have super-high priority... not with Moq v5 (with its superior architecture & approqch to code generation) just around the corner.

But if you'd like to work on it anyway, I guess it would be good to do this in stages, i.e.:

  1. Start with the last bit, i.e. work on Ability to specify proxy generator or invalidate it #925 as mentioned in Ability to specify proxy generator or invalidate it #925 (comment).

  2. Then define the public API for ProxyFactory and open up that base class for subclassing.

  3. Implement your Cecil-based ProxyFactory and publish as a separate contrib NuGet package.

@Znurre
Copy link

Znurre commented Oct 29, 2020

Thank you so much for the pointers @stakx.

We will most likely not be able to upgrade to Moq v5 anytime soon due to legacy code base, so we will make an attempt at creating and upstreaming this alternative ProxyFactory implementation for Moq v4 as per your suggestions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants