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 Improvement - Selective In Domain Execution #21

Open
friendly-source-bot opened this issue Jun 26, 2020 · 14 comments
Open

Performance Improvement - Selective In Domain Execution #21

friendly-source-bot opened this issue Jun 26, 2020 · 14 comments

Comments

@friendly-source-bot
Copy link

Issue created by @rubenhak as Bitbucket Issue #​21 on 2016.10.25 01:37.
Hi,

First of all want to tell you that NUnit.ApplicationDomain solves a big problem with isolating testing environment. I've been using it for couple of weeks now and it was very helpful. Now I'm hitting a different kind of issue - performance. I heavily rely on TestFixtures and TestCasses that already produced 4K test cases. I've noticed that creation and unload of 4K AppDomains increases the test run time from 2 minutes to 20 minutes. Here is my proposal:

In some cases full app domain isolation is not needed. I designed my software so that there is almost no global state, and the global state I can isolate internally using "some magic namespaces". But when test case instances are holding lots of state that is specific to individual test run. So for every test run within the test fixture I needed a separate instance, just like that way it is done in NUnit.ApplicationDomain. So, I'm thinking about a way to reuse the work done and for certain tests still create new instance of test object, but execute it in the default app domain.

I put together a prototype by just commenting out AppDomain creation, instanciated InDomainTestMethodRunner and just executing the methodData. One extra trick was needed. The test instance should have information whether it is a original instance, or the cloned one. One way is to implement the interface:

    public interface IReusableTestFixture
    {
        void MarkAsExecutableTest(bool isExecutable);
    }

And after creating the copy:

      object instance;
      if (testMethodInfo.FixtureArguments == null)
      {
        instance = Activator.CreateInstance(typeUnderTest);
      }
      else
      {
        instance = Activator.CreateInstance(typeUnderTest, testMethodInfo.FixtureArguments);
      }

      if (instance is NUnit.Framework.IReusableTestFixture)
      {
         ((NUnit.Framework.IReusableTestFixture)instance).MarkAsExecutableTest(true);
      }

The text would return from [Init] and [CleanUp] if the call was make for the instance which was not set to be Executable.

Basically this is the rough idea. It would be very nice to have this functionality. Let me know if you see value in this and we can collaborate.

Cheers,
Ruben

@friendly-source-bot
Copy link
Author

On 2016.10.25 01:37 @rubenhak modified issue:
title changed Performance Improvement - In Domain ExecutionPerformance Improvement - Selective In Domain Execution

@friendly-source-bot
Copy link
Author

On 2016.10.25 21:17, @zastrowm commented:
I think I understand the problem (the overhead of appdomains is too high), but I'm a little unclear what you're proposing.

Are you proposing:

  1. AppDomains be re-used for multiple tests
  2. Test instances be re-used for multiple tests
  3. A way to share data between multiple tests/domains
  4. Something else

Feel free to choose more than one :: )

@friendly-source-bot
Copy link
Author

On 2016.10.25 21:27, @rubenhak commented:
Actually, I'm proposing to

  1. The default AppDomain to be used. Don't see much value in reusing created AppDoamins since there will not be complete isolation. Then why just not to use the default one.
  2. A new test instance to be created.

I personally do not see a need for sharing data across tests, but if needed, then the current method of sharing data should work just well.

And of course, all above to be a selective behavior, maybe with some attribute, or an argument to RunInApplicationDomain attribute.

@friendly-source-bot
Copy link
Author

On 2016.10.26 20:36, @zastrowm commented:
So odd question, but if you don't want separate AppDomains, why are you using the RunInApplicationDomainAttribute in the first place?

@friendly-source-bot
Copy link
Author

On 2016.10.26 21:17, @rubenhak commented:
The question totally makes sense. In reality I needed new instance of test object. RunInApplicationDomainAttribute covers my need. But it does a lot more than that - it also creates a new AppDomian. You had put a lot of effort in building the RunInApplicationDomainAttribute, so I naturally thought to have an extension since many of the code could be reused when the there is no need to use a new AppDomain and just new test instance is sufficient. From design perspective it may be even a different RunInNewTestInstanceAttribute or something similar. If you think this is well beyond the scope of the NUnit.ApplicationDomain project I will definitely understand.

@friendly-source-bot
Copy link
Author

friendly-source-bot commented Jun 26, 2020

On 2016.10.30 23:31, @zastrowm commented:
Quick Update: I'm still considering the feature.

I wouldn't be opposed to including something that allows the app-domain to be customized, but only if someone who is using app domains would want to use the feature.

I am currently leaning along the lines of what you said, specifying a type to actually construct the appdomain:

[RunInAppDomain(AppDomainFactory=typeof(MyDomainFactory))]
public void MyTest()
{
  
}
...
public class MyDomainFactory : IAppDomainFactory
{
  AppDomain IAppDomainFactory.GetAppDomainFor(Test test, ...)
  {
  	...    
  }
  
  void IAppDomainFactory.MarkTestFinished(AppDomain domain, Test test, ...)
  {
    ...
  }
}

Would that work for you?

Also, is the reason that you want a new test instance every time so that the constructor always runs? If so, you could also use reflection to re-invoke the constructor (I'm just mentioning it in case it does what you want).

@friendly-source-bot
Copy link
Author

On 2016.10.30 23:42, @rubenhak commented:
My tests are asynchronous internally and they hold a state. When running test in parallel the state in the test instances should be isolated. The most straightforward way seems to create a new instance of a test instance. I don't think reinvoking the constructor would help since the same instance would be used when running tests in parallel.

@friendly-source-bot
Copy link
Author

On 2016.10.30 23:44, @rubenhak commented:
Not much related to the discussion above, but wanted to share that I found that if you rethrow the exception using the following method below, the stack trace would be more useful and interactive in the visual studio runner:

ExceptionDispatchInfo.Capture(ex.InnerException).Throw();

@friendly-source-bot
Copy link
Author

On 2016.11.14 22:16, @zastrowm commented:
@rubenhak yes, using Capture should work. In fact, I'm confused why I don't already have that unless it didn't work for some reason; I'll have to investigate

@friendly-source-bot
Copy link
Author

friendly-source-bot commented Jun 26, 2020

On 2016.11.14 22:19, @zastrowm commented:
@rubenhak What did you think of the previous example? Yours would end up looking like:

public class MyDomainFactory : IAppDomainFactory
{
  AppDomain IAppDomainFactory.GetAppDomainFor(Test test, ...)
  {
    return AppDomain.Current;
  }

  void IAppDomainFactory.MarkTestFinished(AppDomain domain, Test test, ...)
  {
    // no need to do anything, it's already ready
  }
}

In short, it's a way to provide "custom" app-domains via whatever mechanism the developer chooses, which is why I like it.

@friendly-source-bot
Copy link
Author

On 2017.07.26 22:23, @zastrowm commented:
waiting for a response. will close eventually otherwise...

@friendly-source-bot
Copy link
Author

On 2017.07.26 22:23 @zastrowm modified issue:
status changed newon hold

@friendly-source-bot
Copy link
Author

On 2017.07.26 23:34, @rubenhak commented:
@zastrowm, I like the MyDomainFactory interface. It looks pretty reasonable.
P.S. sorry for delayed response. Got pulled into something else.

@OwnageIsMagic
Copy link

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

No branches or pull requests

2 participants