-
-
Notifications
You must be signed in to change notification settings - Fork 801
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
Find out why Moq's performance has become worse since version approx. 4.5 #504
Comments
I noticed this since we're using |
@toha73: Have you by change done any profiling and found out why that is? Do you possibly have some code / use cases you could share to be used for profiling? |
I just did a simple script in LinqPad to test it out between different versions of Moq.
And the results on my machine are: Moq 4.7.142
Moq 4.7.137
Moq 4.7.127
Moq 4.7.99
Moq 4.7.63
|
Ran a quick benchmark comparing with NSubstitute BenchmarkDotNet=v0.10.9, OS=Windows 7 SP1 (6.1.7601)
Processor=Intel Xeon CPU E5-2637 v4 3.50GHzIntel Xeon CPU E5-2637 v4 3.50GHz, ProcessorCount=16
Frequency=3410107 Hz, Resolution=293.2459 ns, Timer=TSC
[Host] : .NET Framework 4.7 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.2116.0
DefaultJob : .NET Framework 4.7 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.2116.0
| Method | Mean | Error | StdDev | Scaled | ScaledSD | Gen 0 | Gen 1 | Allocated |
|----------------------------- |-------------:|-----------:|-----------:|-------:|---------:|--------:|-------:|----------:|
| SubstituteForFoo | 7.754 us | 0.3237 us | 0.9543 us | 1.00 | 0.00 | 1.0529 | 0.0229 | 6.5 KB |
| SubstituteForFooValueReturns | 14.580 us | 0.2907 us | 0.8105 us | 1.91 | 0.26 | 1.7700 | 0.0305 | 10.94 KB |
| MockOfIFoo | 892.996 us | 19.8067 us | 57.7770 us | 116.97 | 16.61 | 7.8125 | 3.9063 | 50.28 KB |
| MockOfIFooWithExpression | 2,138.479 us | 41.3511 us | 44.2452 us | 280.10 | 35.83 | 11.7188 | 3.9063 | 80.59 KB | public class MoqBenchmarks
{
public interface IFoo
{
int Value { get; }
}
[Benchmark(Baseline = true)]
public object SubstituteForFoo()
{
return Substitute.For<IFoo>();
}
[Benchmark]
public object SubstituteForFooValueReturns()
{
return Substitute.For<IFoo>().Value.Returns(1);
}
[Benchmark]
public object MockOfIFoo()
{
return Mock.Of<IFoo>();
}
[Benchmark]
public object MockOfIFooWithExpression()
{
return Mock.Of<IFoo>(x => x.Value == 1);
}
} |
@toha73, thanks for the example. I ran your code for the same versions you mentioned, and also a slightly adapted version with BenchmarkDotNet. Below are my results for each run. TL;DR: I do not observe that extreme jump (by more than an order of magnitude) that you saw. Do you by chance have .NET 4.7(.1) installed? If so, are you aware of the performance problems caused by it? (See microsoft/dotnet#529 and other issues in the same repo.)
4.7.142Using your code:
Using BenchmarkDotNet:
4.7.99Using your code:
Using BenchmarkDotNet:
4.7.63Using your code:
Using BenchmarkDotNet:
|
@stakx Can we have a short private discussion, perhaps you can ping me on gitter? https://gitter.im/DotNetAnalyzers/IDisposableAnalyzers |
@toha73 - FYI: If you upgrade to Moq 4.7.145 (or later), which is no longer affected by the mentioned .NET Framework regression (unless you opt back in), execution times should go back down to a normal level. See the changelog for details. |
I've spent some time with PerfView. It seems that the biggest performance bottlenecks are:
Those cannot be made more efficient, we can only try to do less of it. Notable hotspots in Moq's codebase are:
Moq's performance would likely be greatly improved by two changes:
Of these two changes, (2) would definitely be a breaking change (but worth it). (1) might be done as a non-breaking change if much care is taken. |
Oh now I can comment it. I couldn`t comment it earlier. |
@jakubozga - Yes, work is being done to make Moq faster. I've identified a few ways to significantly improve performance related to the major bottleneck mentioned above, but implementing these improvements will take a lot of care and time; Moq's functional correctness takes precedence over performance. I've already applied quite a few smaller-scale improvements to both runtime and memory efficiency, but those will probably only become noticeable when you have many thousands of unit tests. The major performance bottlenecks mentioned in my above post often result from fundamental architectural decisions, and it's not trivial to remove them without causing collateral damage. For example, I have this wonderful draft re-implementation that makes I don't make any promises about speed or memory efficiency improvements in particular versions—I'd advise common sense: Just try out new versions as they're being released, and if they work well for you, then that's great. |
@jakubozga - I have just found and removed a major, but unexpected performance bottleneck (see referenced PR above). I am fairly confident that Moq 4.8.0 will be noticeably faster than its predecessor versions. Performance should once again be comparable to versions earlier than 4.5.16. Moq's own unit test suite runs nearly twice as fast now (12.2 seconds instead of 20.2 seconds on the CI server). |
Actually here is FastExpressionCompiler wich speed-ups I have just fixed the last three failing tests from Moq with all |
@dadhi - That's some impressive work there! 👍 Given that Moq 4.8+'s performance is once again comparable to what it used to be in early 4.x versions, I feel that further performance improvements are less of a priority now than approx. half a year ago. At this point in Moq's lifecycle (shortly before Moq 5's release), I think stability and bug fixing is generally the more important goal. Because of that, I would prefer if Moq 4 did not take on any further hard dependencies on third-party libraries that could introduce new breaking changes. (A small aside: I've learnt the hard way that the code coverage of Moq's unit test suite isn't nearly as complete as it should be, so in your work, I wouldn't rely too heavily on it.) That being said, what we could do is to introduce an extensibility point that would allow swapping out the expression tree compiler used by Moq. Moq would default to the framework's own Does that sound fair to you? Btw., there are still some remaining options for improving performance further:
|
@dadhi, I did some quick prototyping of an extensibility point for custom expression tree compilers, see #647. This would allow you to do this: // using Moq;
ExpressionCompiler.Instance = new CacheExpressionCompiler(new AlternateExpressionCompiler());
// this implementation might e.g. use your FastExpressionCompiler library:
class AlternateExpressionCompiler : ExpressionCompiler
{
public override Delegate Compile(LambdaExpression expression) { … }
public override TDelegate Compile<TDelegate>(Expression<TDelegate> expression) { … }
}
// this implementation might cache compiled expressions for later reuse:
class CacheExpressionCompiler : ExpressionCompiler
{
public CacheExpressionCompiler(ExpressionCompiler underlying) { … }
public override Delegate Compile(LambdaExpression expression) { … }
public override TDelegate Compile<TDelegate>(Expression<TDelegate> expression) { … }
} Any thoughts? (Would this be enough to integrate with your library?) /cc @informatorius (FYI because of #188 (comment)) |
I think this is good opt-in feature. Less risky as well. The extensibility api as in prototype is fine too.
Btw, FEC is just a single cs file. You can drop it in your project. This is exactly how I have tested it with Moq. The only change was a replacing all
May you do this already, but in my case, I am gradually adding issue covering tests into solution with each new issue. I have a separate test project for that, which include 3rd party references for real use-cases (issues). |
Glad to hear! I might actually merge this, then.
Understood. And apologies if I didn't express myself clearly enough. By "dependencies on third-party libraries", I didn't just mean the form of distribution. Whether it is a NuGet package, or a single source code file doesn't matter all that much. (After all, you could easily use NuGet to distribute your code file as a development-time, source-code only NuGet package, of which Moq already references a few.) It is more the thought of replacing a framework facility of significant complexity with a reimplementation that very likely won't be nearly as "battle-tested" (hardened against bugs and refined over time) as the framework code is, due to its comparatively short history. (No disrespect intended!) So I think swapping out the expression compiler should be an explicit opt-in decision made by downstream users; hence the proposed extensibility point.
(I have been thinking about setting up a regression test suite that goes beyond Moq's own regression tests. For example, I'd like to run some actual real-world open-source projects' test suites before and after some change in Moq, and see if the two test runs produce different results. If so, that might be an indication that the change causes a regression. Haven't got around to that yet, though.) |
All is fine with me, cause I've just got another ~1000 tests (Moq's) to FEC regression suite, in addition to the biggest tandem of DryIoc, and hmm.. AutoMapper :) |
It was noted e.g. in #388 (comment) that Moq's performance has suffered in recent releases. It would be nice to find the reason(s) for this, and perhaps apply some optimizations to start reversing that trend before version 4.8.0 gets released.
Suggestion:
/cc @jatouma, @JohanLarsson
The text was updated successfully, but these errors were encountered: