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

Adding a setup looks like it became an O(n*n) operation since #984 was merged #1110

Closed
CeesKaas opened this issue Nov 27, 2020 · 6 comments · Fixed by #1111
Closed

Adding a setup looks like it became an O(n*n) operation since #984 was merged #1110

CeesKaas opened this issue Nov 27, 2020 · 6 comments · Fixed by #1111
Milestone

Comments

@CeesKaas
Copy link
Contributor

I don't think this will be an issue for many users but in one of our tests we added 3000 setups to a mock and after updating from 4.13.1 to 4.15.2 the test time went from <10 seconds to over 14 minutes

@stakx
Copy link
Contributor

stakx commented Nov 27, 2020

What led you to the assumption that this slowdown was caused by #984, if I may ask? Understanding this might help looking into that matter.

@CeesKaas
Copy link
Contributor Author

in public void Add(Setup setup) of SetupCollection the call this.MarkOverriddenSetups(); was added

that method loops through all items already in the SetupCollection

@CeesKaas
Copy link
Contributor Author

CeesKaas commented Nov 27, 2020

for a very basic reproduction i used :

using System;
using System.Diagnostics;
using Moq;

namespace moq_repro
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine($"{DateTime.Now:o} Start");
            var sw = Stopwatch.StartNew();
            var mock = new Mock<ITestInterface>();
            for (int i = 0; i < 3000; i++)
            {
                mock.Setup(m=>m.Get(Guid.NewGuid()));
            }
            sw.Stop();
            Console.WriteLine($"{DateTime.Now:o} Finish (took:{sw.Elapsed})");
        }
    }
    public interface ITestInterface{
        void Get(Guid id);
    }
}

with Moq version 4.13.1 this program takes 273ms (on my machine)
with Moq version 4.14.0 it takes 6:28 minutes

@stakx
Copy link
Contributor

stakx commented Nov 27, 2020

OK thanks. I'll look into it.

the call this.MarkOverriddenSetups(); was added

that method loops through all items already in the SetupCollection

Wouldn't that make it 2n (as opposed to n2)?

@CeesKaas
Copy link
Contributor Author

you could be correct i'm not fully up to speed with the big O notation i had 2 loops in my mind (the for loop around the call to setup and the for loop in MarkOverriddenSetups) so the setup add went from O(1) to O(n) probably

@CeesKaas
Copy link
Contributor Author

I've create PR #1111 with a potential fix

@stakx stakx added this to the 4.15.3 milestone Dec 28, 2020
@stakx stakx modified the milestones: 4.15.3, 4.16.0 Jan 1, 2021
mburumaxwell pushed a commit to faluapp/falu-dotnet that referenced this issue Jun 12, 2021
Bumps [Moq](https://github.com/moq/moq4) from 4.15.2 to 4.16.0.

#Changelog

*Sourced from [Moq's changelog](https://github.com/moq/moq4/blob/main/CHANGELOG.md).*

> ## 4.16.0 (2021-01-16)
>
> #### Added
>
> * Ability to directly set up the `.Result` of tasks and value tasks, which makes setup expressions more uniform by rendering dedicated async verbs like `.ReturnsAsync`, `.ThrowsAsync`, etc. unnecessary:
>
>    ```diff
>    -mock.Setup(x => x.GetFooAsync()).ReturnsAsync(foo)
>    +mock.Setup(x => x.GetFooAsync().Result).Returns(foo)
>    ```
>
>    This is useful in places where there currently aren't any such async verbs at all:
>
>    ```diff
>    -Mock.Of<X>(x => x.GetFooAsync() == Task.FromResult(foo))
>    +Mock.Of<X>(x => x.GetFooAsync().Result == foo)
>    ```
>
>    This also allows recursive setups / method chaining across async calls inside a single setup expression:
>
>    ```diff
>    -mock.Setup(x => x.GetFooAsync()).ReturnsAsync(Mock.Of<IFoo>(f => f.Bar == bar))
>    +mock.Setup(x => x.GetFooAsync().Result.Bar).Returns(bar)
>    ```
>
>    or, with only `Mock.Of`:
>
>    ```diff
>    -Mock.Of<X>(x => x.GetFooAsync() == Task.FromResult(Mock.Of<IFoo>(f => f.Bar == bar)))
>    +Mock.Of<X>(x => x.GetFooAsync().Result.Bar == bar)
>    ```
>
>    This should work in all principal setup methods (`Mock.Of`, `mock.Setup…`, `mock.Verify…`). Support in `mock.Protected()` and for custom awaitable types may be added in the future. (@stakx, [#1126](devlooped/moq#1126))
>
> #### Changed
>
> * Attempts to mark conditionals setup as verifiable are once again allowed; it turns out that forbidding it (as was done in [#997](devlooped/moq#997) for version 4.14.0) is in fact a regression. (@stakx, [#1121](devlooped/moq#1121))
>
> #### Fixed
>
> * Performance regression: Adding setups to a mock becomes slower with each setup (@CeesKaas, [#1110](devlooped/moq#1110))
>
> * Regression: `mock.Verify[All]` no longer marks invocations as verified if they were matched by conditional setups. (@Lyra2108, [#1114](devlooped/moq#1114))

#Commits

- [`74d5863`](devlooped/moq@74d5863) Update version to 4.16.0
- [`424fe31`](devlooped/moq@424fe31) Fix typo in changelog
- [`f48c0f4`](devlooped/moq@f48c0f4) Merge pull request [#1126](devlooped/moq#1126) from stakx/setup-task-result
- [`6f6a89d`](devlooped/moq@6f6a89d) Update the changelog
- [`66bcb21`](devlooped/moq@66bcb21) Enable `task.Result` in delegate-based setup methods
- [`42521c4`](devlooped/moq@42521c4) Add ability in `IAwaitableFactory` to create result...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants