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

Create separate proxy generator for each type #938

Closed
wants to merge 1 commit into from

Conversation

ishatalkin
Copy link
Contributor

@ishatalkin ishatalkin commented Oct 15, 2019

There is a known problem of .net that multiple types in one dynamic assembly is way slower than multiple dynamic assemblies with one type each (see stackoverflow). Moq uses Castle Dynamic Proxy to create proxy-types. Unfortunately, it takes more and more time to create each next proxy-type. There is a simple solution - create a dynamic assembly for each proxy type.

The simple test shows that Moq needs 32.9 sec to create 735 mocks of different types. If using one assembly for each proxy type it's necessayr 5.7 sec to create the same 735 mocks, so the speed increases in 5-6 times.

Test:

    public void MultipleObjectCreationTest()
    {
        var sw = new Stopwatch();
        sw.Start();
        int i = 1;
        foreach (var type in AppDomain.CurrentDomain.GetAssemblies()
            .SelectMany(a => a.GetTypes())
            .Where(t => !t.ContainsGenericParameters)
            .Where(t => !t.IsSealed)
            .Where(t => t.IsPublic)
            .Where(t => t.GetConstructor(Type.EmptyTypes) != null))
        {
            try
            {
                var sw2 = new Stopwatch();
                sw2.Start();
                var mockType = typeof(Mock<>).MakeGenericType(type);
                var mock = Activator.CreateInstance(mockType);
                var obj = mockType.GetProperties().Where(p => p.Name == "Object").First().GetValue(mock);
                sw2.Stop();
                Console.WriteLine($"{i}. Type = {type.Name}. Time = {sw2.ElapsedMilliseconds} ms");
                i++;
                if (sw.Elapsed.Minutes > 2)
                    break;
            }
            catch (Exception e)
            {
                Console.WriteLine($"{i}. Type = {type.Name}. Exception = {e}");
            }
        }
        Console.WriteLine($"Time = {sw.Elapsed.TotalSeconds} s, Mocks created = {i - 1}");
    }

In our project we increased the overall time of unit tests from 19 to 2 minutes using one assembly for one proxy type approach.

@ishatalkin
Copy link
Contributor Author

Time necessary to create one proxy (all proxy types in one assembly - this is the current Moq behaviour)
X - the number of proxy
Y - time to create the proxy (ms)

EXCEL_Bg6tMkljqv

@ishatalkin
Copy link
Contributor Author

Time necessary to create one proxy (one assembly for one proxy type approach)
X - the number of proxy
Y - time to create the proxy (ms)

EXCEL_MMANIGwyif

@ishatalkin ishatalkin changed the title Create separate generator for each type Create separate proxy generator for each type Oct 15, 2019
@stakx
Copy link
Contributor

stakx commented Oct 15, 2019

Hi @ishatalkin, and thank you for contributing! This is intriguing, however I am not going to merge this PR. While I agree that proxy generator creation might currently be sub-optimal in certain scenarios, I don't think newing up fresh ProxyGenerators by default is the way to go. Reusing a single generator should be perfectly fine for most regular use cases.

What's lacking in all of these measurements is statements about memory consumption. Dynamic assemblies don't come for free, and currently DynamicProxy lacks the ability to create collectible ones (see castleproject/Core#473), so I would have to assume that once created, they're going to remain in memory (unless you unload a whole app domain, which may not even be possible on .NET Core <3). If your unit test suite ran for 19 seconds and that time was mostly because of proxy creation, I'm estimating that you must be creating tens of thousands of proxies there. Switching to one dynamic assembly per proxy type would likely yield a similarly staggering amount of assemblies loaded into the app domain... that can't be good with regard to memory usage. (But I admit that measurements would be required to back that up.) Nonetheless, note that DynamicProxy explicitly recommends using a single generator (see https://github.com/castleproject/Core/blob/master/docs/dynamicproxy.md).

The more important reason for rejecting this PR, however, is my belief that we can do better by giving user code more explicit control over proxy generator instantiation; see #925. The solution proposed there should work for your scenario, too, while avoiding the cons of forced increased memory consumption & cache misses in DP.

@stakx stakx closed this Oct 15, 2019
@ishatalkin
Copy link
Contributor Author

If #925 will give the ability to create a proxy generator for each type, it's OK.

About memory usage: I compared allocated memory in both situations, and it's almost the same (it differs < 1%). What is essential: GC needs some more time for its operations:

  • 0.4% of overall time with standart Moq;
  • 3.8% - with my customization.

Nevertheless, the overall time of our tests get down.

@Timurzen
Copy link

Useful changes, I would like to see in the release

@stakx
Copy link
Contributor

stakx commented Oct 15, 2019

If #925 will give the ability to create a proxy generator for each type, it's OK.

Yes, but not automatically. Given that the proposal gets merged (I have a draft implementation lying around & waiting btw.) it would be your task to decide whether a mock should use a non-default factory, and if so, which one. Selecting a (or newing up a fresh) proxy factory per mocked type would only be one out of many possible policies.

The only built-in policy would be that new mocks use a factory that's been set as the default (either globally, or at the MockRepository level), unless told otherwise.

Btw. two Mock<T> with the same T don't necessarily end up using the same proxy type. You'd probably want to consider additional interface types (specified using mock.As<TInterface>()) along with the main mocked type.

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

Successfully merging this pull request may close these issues.

None yet

3 participants