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

Performance with large interfaces #1128

Closed
michael-baker opened this issue Jan 7, 2021 · 4 comments
Closed

Performance with large interfaces #1128

michael-baker opened this issue Jan 7, 2021 · 4 comments

Comments

@michael-baker
Copy link

michael-baker commented Jan 7, 2021

When upgrading from Moq 4.10.0 to 4.15.2 and mocking an interface with thousands of methods the cost of creating the runtime mock is significant which slows down test runs. Yes I know that god objects are a code smell.... :)

The most expensive call is GetInterfaceMap via GetImplementingMethod: https://github.com/moq/moq4/blob/9fcf8a0c214dc6cdcaee813b0623e7c3f121924c/src/Moq/Extensions.cs
which is called repeatedly.

When creating a mock of my type the test execution time for a single test is around 5 seconds.
After adding a simple fix to cache the results of GetInterfaceMap the execution time drops to milliseconds.

Would adding a cache for the results of GetInterfaceMap be accepted? As far as I can fathom the results of the call cannot change given that the input type is already baked.

The method is an extension method and has no access to any sort of Moq internals so I'm stumped on where to put a cache that is harmonious with the the design of Moq. I have fork that just caches the GetInterfaceMap calls in the extension method itself but this feels unclean.

@stakx
Copy link
Contributor

stakx commented Jan 8, 2021

Your use case might have revealed a potential performance problem, but having an interface with thousands of methods isn't exactly the most common use case (how on Earth would you ever implement that monstrosity by hand?!) so I don't think we should rush a cache into Moq without taking a closer look. A cache should be among the last options to consider, not the first.

IIRC, GetImplementingMethod doesn't enter the picture until you add setups to your mock. If you just create a mock, the only one calling GetInterfaceMap should be DynamicProxy (the proxying library on top of which Moq 4 is built). Proxying gigantic types is known to have potentially bad performance, not much we can do about this.

Can you please provide a runnable repro?

@stakx stakx closed this as completed Feb 2, 2021
@rauhs
Copy link
Contributor

rauhs commented Jul 10, 2023

Can we revisit this?

We're seeing the exact same issue. For us, a few tests spend 99% of the time in just the Moq. This means we had to disable a few performance tests.

Here is a quick PerfView trace of the runtime:

Name                                                                                                                                                    	Inc %	      Inc	Exc %	Exc	Fold	                             When	      First	       Last
 castle.core!Castle.DynamicProxy.AbstractInvocation.Proceed()                                                                                           	 98.7	   13,601	  0.0	  0	   0	 09999999999999999999999999999999	 10,774.284	 24,810.652
+ moq!Moq.CastleProxyFactory+Interceptor.Intercept(class Castle.DynamicProxy.IInvocation)                                                               	 98.7	   13,601	  0.0	  1	   0	 99999o99999999999999999999999999	 10,774.284	 24,810.652
 + moq!Moq.Mock.Moq.IInterceptor.Intercept(class Moq.Invocation)                                                                                        	 98.7	   13,597	  0.0	  0	   0	 99999o99999999999999999999999999	 10,774.284	 24,810.652
 |+ moq!FindAndExecuteMatchingSetup.Handle                                                                                                              	 98.7	   13,594	  0.0	  1	   0	 o9999999999999999999999999999999	 10,774.284	 24,810.652
 ||+ moq!SetupCollection.FindLast                                                                                                                       	 98.6	   13,585	  0.0	  0	   0	 o9999999999999999999999999999999	 10,774.284	 24,810.652
 |||+ moq!Moq.Setup.Matches(class Moq.Invocation)                                                                                                       	 98.6	   13,585	  0.0	  0	   0	 o9999999999999999999999999999999	 10,774.284	 24,810.652
 ||| + moq!Moq.MethodExpectation.IsMatch(class Moq.Invocation)                                                                                          	 98.6	   13,584	  0.0	  0	   0	 o9999999999999999999999999999999	 10,774.284	 24,810.652
 ||| |+ moq!Moq.MethodExpectation.IsOverride(class Moq.Invocation)                                                                                      	 98.6	   13,583	  0.0	  0	   0	 o9999999999999999999999999999999	 10,774.284	 24,810.652
 ||| ||+ moq!Moq.Invocation.get_MethodImplementation()                                                                                                  	 97.1	   13,387	  0.0	  0	   0	 o9899999999989999898999989999999	 10,776.284	 24,810.652
 ||| |||+ moq!Extensions.GetImplementingMethod                                                                                                          	 97.1	   13,387	  0.0	  0	   0	 o9899999999989999898999989999999	 10,776.284	 24,810.652
 ||| ||| + mscorlib.ni!RuntimeType.GetInterfaceMap                                                                                                      	 97.0	   13,368	  0.1	 15	   0	 o9899999999989899898999989999999	 10,776.284	 24,810.652
 ||| ||| |+ mscorlib.ni!RuntimeType.GetMethodBase                                                                                                       	 49.2	    6,773	  0.3	 35	   0	 o5444554444444454444444545445454	 10,776.284	 24,810.652
 ||| ||| |+ mscorlib.ni!DomainNeutralILStubClass.IL_STUB_PInvoke(System.RuntimeTypeHandle, System.RuntimeTypeHandle, System.RuntimeMethodHandleInternal)	 44.8	    6,177	  0.2	 27	   0	 o4444444434444344444443434444344	 10,787.290	 24,808.652
Name

@stakx

This comment was marked as outdated.

@stakx
Copy link
Contributor

stakx commented Jul 20, 2023

Scratch my above comment, I only noticed just now that you reposted this in a new issue. Let's continue there.

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