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

Substitute.For delegate much slower than for interfaces and classes #248

Closed
jcansdale opened this issue Sep 9, 2016 · 6 comments · Fixed by #362
Closed

Substitute.For delegate much slower than for interfaces and classes #248

jcansdale opened this issue Sep 9, 2016 · 6 comments · Fixed by #362
Labels
performance Issues related to speed/memory use

Comments

@jcansdale
Copy link

jcansdale commented Sep 9, 2016

First, congratulations on a very slick project. :)

I've been doing some performance tests using BenchmarkDotNet and noticed that substituting delegates is much slower than for other types (something like 80 times slower). I wondered if you know why this might be and if it could be improved?

Here are the results and benchmarking code:

Host Process Environment Information:
BenchmarkDotNet.Core=v0.9.9.0
OS=Microsoft Windows NT 6.2.9200.0
Processor=Intel(R) Core(TM) i7-4500U CPU 1.80GHz, ProcessorCount=4
Frequency=2338341 ticks, Resolution=427.6536 ns, Timer=TSC
CLR=MS.NET 4.0.30319.42000, Arch=32-bit ?
GC=Concurrent Workstation
JitModules=clrjit-v4.6.1586.0

Type=NSubstituteBenchmark  Mode=Throughput  LaunchCount=1  
WarmupCount=3  TargetCount=3  
Method Median StdDev Scaled Scaled-SD Min Max
SubstituteForInterface 6.3367 us 0.0920 us 1.00 0.00 6.2362 us 6.4199 us
SubstituteForClass 6.5393 us 0.0584 us 1.03 0.01 6.4411 us 6.5448 us
SubstituteForDelegate 545.3938 us 53.6336 us 83.17 6.99 465.9712 us 568.1222 us
namespace StaticMocks.Benchmarks
{
    using System;
    using NSubstitute;
    using BenchmarkDotNet.Attributes;
    using BenchmarkDotNet.Attributes.Columns;

    [MinColumn, MaxColumn]
    public class NSubstituteBenchmark
    {
        [Benchmark]
        public object SubstituteForInterface()
        {
            return Substitute.For<IServiceProvider>();
        }

        [Benchmark]
        public object SubstituteForClass()
        {
            return Substitute.For<Type>();
        }

        [Benchmark]
        public object SubstituteForDelegate()
        {
            return Substitute.For<Action>();
        }
    }
}
@jcansdale
Copy link
Author

I'm wondering if the commonly used Action and Func delegates could be special cased? I can imagine that substituting an unknown delegate might be tricky.

@dtchepak
Copy link
Member

Hi Jamie,
Thanks a lot for this. The DelegateProxyFactory does a lot of extra work. I think special cases would help. Possibly adding some caching of the proxy expression too.

@alexandrnikitin
Copy link
Member

It spends most of the time in System.Reflection compiling the delegate
Profiling

There are faster compilers e.g. https://github.com/dadhi/FastExpressionCompiler but I'm not sure we should touch it at all. What do you think?

@jcansdale
Copy link
Author

I now have a workaround that lets me substitute an interface rather than a delegate (which is ~80 times faster).

It looks something like this:

        static Delegate createDelegateFor(Type delegateType)
        {
            var dele = quickCreateDelegateFor(delegateType);
            if(dele != null)
            {
                return dele;
            }

            return (Delegate)Substitute.For(new Type[] { delegateType }, new object[0]);
        }

        // TODO: Add support for all Actions and Funcs.
        static Delegate quickCreateDelegateFor(Type delegateType)
        {
            Type type;
            switch (delegateType.Name)
            {
                case "Func`1":
                    type = typeof(IFunc<>).MakeGenericType(delegateType.GenericTypeArguments);
                    break;
                case "Func`2":
                    type = typeof(IFunc<,>).MakeGenericType(delegateType.GenericTypeArguments);
                    break;
                default:
                    return null;
            }

            var target = Substitute.For(new Type[] { type }, new object[0]);
            var invokeMethod = type.GetTypeInfo().GetMethod("Invoke");
            return invokeMethod.CreateDelegate(delegateType, target);
        }

        public interface IFunc<R> { R Invoke(); }

        public interface IFunc<T, R> { R Invoke(T t); }

I'm guessing there may be dragons that I'm missing, but this seems to work for my purposes. 😄

@dtchepak dtchepak added the performance Issues related to speed/memory use label Oct 6, 2017
@zvirja
Copy link
Contributor

zvirja commented Feb 27, 2018

I've investigated this issue a bit and basically I see two options:

  1. Use the FastExpressionCompiler mentioned above;
  2. Rewrite code to use the Reflection.Emit directly.

The second option is probably more lightweight, however it will force to have a mess in our code, as it's non-supportable. Also I'm not sure that the emitted code will be 100% valid (so it doesn't fail on some Mono environment), as sometimes opcodes are very specific depending on the operand type (e.g. different opcode is used to read int, long and string argument value).

Therefore, the safest way (and the preferable one) would be to use the FastExpressionCompiler which has been already tested and proved to work. We can import the whole FastExpressionCompiler.cs file to our solution (making the declarations internal), so we will not depend on it directly.

@alexandrnikitin @dtchepak What do you think? 😉

@dtchepak
Copy link
Member

@zvirja Sounds good 👍

dtchepak pushed a commit that referenced this issue Mar 1, 2018
Previously delegates were generated using the expressions, which
proxied all arguments to ICallRouter and later handled the result.
I've found that there is a better way - we can use the standard
Castle intercepting feature. The idea is that our delegate proxy
could be a regular delegate over the Caslte proxy's method.

That allowed to significantly improve the performance, as
expression compilation is pretty expensive.

Workflow:
1. Generate an interface for the requested delegate type, so it
contains the "Invoke" method with the required signature.
2. Ask Castle Proxy to generate a proxy for that interface.
3. Create a delegate from the "Invoke" method of the generated proxy.

It appears that this change is almost transparent for the engine
and works perfectly.

Fixes #248.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues related to speed/memory use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants