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

Moq 64bit bad performance #188

Closed
informatorius opened this issue Jul 1, 2015 · 15 comments
Closed

Moq 64bit bad performance #188

informatorius opened this issue Jul 1, 2015 · 15 comments

Comments

@informatorius
Copy link
Contributor

Since I build all targets in 64bit instead of 32bit, the unit test duration has doubled because Moq is 50% slower in 64bit. Can you confirm this problem?
Any ideas how to enhance performance in 64bit?

@informatorius
Copy link
Contributor Author

I found .net 4.6 RyuJIT improves performance by 20%

@informatorius
Copy link
Contributor Author

I did some performance profiling for a sample unittest with x86 and x64.
As you can see in the attached screenshots x64 takes two times x86.
profile moq unittest x86
profile-86
profile moq unittest x64
profile-64

@informatorius
Copy link
Contributor Author

This is the whole trace if I did it right. The MessageBox.Show was just a trigger for me take the profiler snapshot. As there is no CastleProxy code here it is a Moq only problem. I believe it is the LambdaCompiler.Compile from .Net which is slower in 64bit

My TestCode defines an Interface with 40 methods and creates an InterfaceMock using Moq.
Then for each method a Moq Setup is called in DoFurtherSetups.
The MessageBox.Show which holds until closed are my triggers for Profiler attach and capture.

[TestMethod]
public void TestMethod5()
{
    MessageBox.Show("Attach");
    var interfaceMock = new InterfaceMock();
    interfaceMock.DoFurtherSetups();
    MessageBox.Show("Capture");
}

@kzu
Copy link
Member

kzu commented Sep 2, 2015

Not sure there's anything we can do to make LambdaExpression.Compile any
faster...

@informatorius
Copy link
Contributor Author

@informatorius
Copy link
Contributor Author

I found the compiled lambda expressions can be cached.
http://tips.x-tensive.com/2009/05/fast-expression-compilation.html
I used the Xtensive.Core dll from Nuget and replaced inside Moq the lambda.Compile() calls with lambda.CachedCompile() where feasible.

Now the unit tests execution time in my scenario is half the time!

Can you implement a similar lambda compile cache mechanism or include Xtensive.Core?

@kzu
Copy link
Member

kzu commented Oct 6, 2015

Including that component isn't likely to happen.

Maybe you'd like to take on the challenge to submit a PR with that feature?
I'm sure lots of Moq users and myself will be very grateful :)

@stakx
Copy link
Contributor

stakx commented Jun 28, 2017

Before we start optimizing, we should probably set up some kind of benchmarking (doesn't have to be part of the build process / CI, but we should have some kind of strategy) so we can actually measure the effect of any changes we make; see #388.

@informatorius
Copy link
Contributor Author

informatorius commented Feb 23, 2018

I implemented lambda expression compile cache. It is quite easy and performance increases for the Mock.Of() calls.

Moq needs a static class LambdaCompileCache for extension method lambda.CachedCompile();

Then all calls to lambda.Compile() in Moq code (6) must be replaced by lambda.CachedCompile()

The Moq ExpressionComparer needs to be modified so that GetHashCode() returns not expression.GetHashCode() but expression.ToString().GetHashCode()

namespace Moq
{
	internal static class LambdaCompileCache
	{
		public static readonly IDictionary<Expression, object> Dictionary = new ConcurrentDictionary<Expression, object>(ExpressionEqualityComparer.Instance);

		public static int HitCounter;
		public static int MissCounter;

		public static T CachedCompile<T>(this Expression<T> lambda)
		{
			T lambdaCompile;
			object obj;

			bool found = Dictionary.TryGetValue(lambda, out obj);

			if (found)
			{
				HitCounter++;

				lambdaCompile = (T)obj;
			}
			else
			{
				MissCounter++;

				lambdaCompile = lambda.Compile();

				Dictionary.Add(lambda, lambdaCompile);
			}

			return lambdaCompile;
		}
	}

	internal class ExpressionEqualityComparer : IEqualityComparer<Expression>
	{
		public static readonly ExpressionEqualityComparer Instance = new ExpressionEqualityComparer();
		
		private ExpressionEqualityComparer()
		{
			// Singleton
		}

		public bool Equals(Expression x, Expression y)
		{
			return ExpressionComparer.Default.Equals(x, y);
		}

		public int GetHashCode(Expression obj)
		{
			var str = obj.ToString();
			return str.GetHashCode();
		}
	}
}

@stakx
Copy link
Contributor

stakx commented Feb 23, 2018

@informatorius, thanks for working on this! Could you share some details on how you measured the performance improvement, and give an idea of how much performance is improved in a typical testing scenario?

I guess if the performance gain is sufficiently big, we could incorporate this into Moq, however before you submit a PR I'd like to mention a few points:

  • Cache without expiry policy == memory leak. This may or may not be an issue depending on users' various usage scenarios. Ideally, a cache would make use of some sort of "weak dictionary" that releases rarely requested entries after a while for GC.

  • Reliability. Lambda compilation happens in some key places in Moq. If it all goes through a cache, it's crucial that there are no false cache hits. That is, ExpressionComparer should be made really bullet-proof, and we'd perhaps need a lot more additional unit tests to verify this.

  • Add to Moq as experimental feature first? As long as we don't have a guarantee that the cache won't produce any false cache hits, the feature should be experimental, and therefore explicitly opt-in. This could be done via Mock.Switches.

  • Thread-safety. You should probably increment HitCounter and MissCounter using Interlocked.Increment.

  • Why compute the hash code through the string representation? Is it really necessary to first build a string representation just to compute a hash code? Why not build a "composite" hash code by traversing the expression tree? That'd likely be much more efficient than allocating strings.

    If you do build a hash code via an expression tree's string representation, it would probably be better to use Moq's own expression.ToStringFixed() extension method to get a (potentially) more accurate string representation.

  • LambdaCompileCache's fields should be private, not public. (And the counters should be exposed through public properties.)

There's probably more, but that's the couple points that came to mind while looking at your above code. If you'd like to submit a PR for this, I'd be happy if you could take the above points into account. :-)

@stakx stakx removed this from the v4.9.0 milestone Feb 26, 2018
@informatorius
Copy link
Contributor Author

informatorius commented Feb 27, 2018

Good points!

Most performance improve I see with lots of Mock.Of<> maybe because it has the most setups internally. One test run with 1000 production unit tests with many Mock.Of<> showed an improvement from runtime 3min to 2min (all with 1 thread sequentially) using Moq 4.8.1. So this is 30%. I see the same when I setup an IMyInterface with 50 properties and methods of type IAnotherInterface and do Mock.Of<> or SetupAllProperties() with DefaultValue=DefaultValue.Mock then setup time goes from 900ms to 600 ms (including testrunner start). But that may not be a typical scenario but it is how our unit tests work.

I will now try to run all production unittests (10000) with cachedcompile moq to see if any tests fail and so breaks moq and how performance changes.

Then I make pull request and we can discuss changes there.

@stakx
Copy link
Contributor

stakx commented Feb 27, 2018

@informatorius - Thank you! It would be great if you could share the results of running your 10K production unit tests against your lambda compiler cache.

@informatorius
Copy link
Contributor Author

I got no performance improvement when using new Mock instead of Mock.Of. So maybe if Mock.Of can be tweaked more then LambdaCachedCompile is not necessary.

@stakx
Copy link
Contributor

stakx commented Feb 27, 2018

@informatorius - I have a few ideas about how to improve performance further. For example, it would be entirely feasible to reduce the amount of (slow) lambda compilation going on simply by re-designing how Moq processes setups.

I originally planned to do this kind of re-design work for Moq 4.9.0, but since it's a fairly large effort, I have postponed this milestone until after Moq 5 is released. My feeling is that Moq 5 will have better performance than Moq 4 right from the start, so it might be better for people to adopt Moq 5 as soon as possible instead of waiting for a faster Moq 4. We'll see. :-)

@stakx
Copy link
Contributor

stakx commented Jul 13, 2018

Closing this dormant issue, but marking it as "unresolved" so it can be easily found again. Please see #642 for details. If you'd like to pick this up and work on it, please post here briefly and we'll see what we can do!

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

No branches or pull requests

3 participants