-
Notifications
You must be signed in to change notification settings - Fork 81
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
DisableAppDomain switch is not fully respected for Discovery #331
Comments
@bradwilson not trying to be pushy, but this would really help us speed up things under VS. Is it okay if I PR fixes for it? |
It's up to @clairernovotny to decide which fixes to take and when releases are made. |
@clairernovotny polite bump on this one. |
@nohwnd FYI @clairernovotny has stepped away from this project, but if you are able to provide a PR, I can put it into the 2.5.0 release (I anticipate a final build happening in the next few weeks). |
I am not sure if I will be able to produce and test it in the next 2 weeks, but I take this as a GO and will look into it when I have some time. |
Coming to a 2.5.0 near you, soon... |
I am testing with Roslyn like solution, to see how much gain we would get by passing DisableAppDomain switch by default in discovery to avoid the slowness of RealProxy xunit/xunit#2323, because we run in parallel by default now, and isolate on process level.
DisableAppDomain vstest option switch was implemented in xunit/xunit#944, but I don't think it is fully propagated for discovery. When I debug the testhost process I can see there are still 4 appdomains. And I can see a lot of time is spent remoting between them.
I am also unsure if CollectSourceInformation is propagated fully, because that I see dia session being created, in a separate appdomain, but maybe it is created but later not used (honoring the option is implemented in xunit/xunit#1352).
When I crudely patch v2 xunit and xunit.runner.visualstudio to always use AppDomainManager_NoAppDomain, and NullSourceInformationProvider, the discovery time goes from ~8 seconds down to ~2 for the biggest projects with ~25k tests each:
Before, ~8s per project:
After ~2s per project:
I don't have a simple repro for the fix, but .NET does not use appdomains (and is overall faster), so creating a solution with 25k tests, and switching between net48, and net6.0 target frameworks shows significantly different times it takes to discover the project under VS.
Would you take a fix for this for v2?
The text was updated successfully, but these errors were encountered: