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

Xunit adapter perf issue in VisualStudio and vstest.console runners #485

Closed
Shyam-Gupta opened this issue Feb 14, 2017 · 5 comments
Closed

Comments

@Shyam-Gupta
Copy link
Member

Shyam-Gupta commented Feb 14, 2017

We recently did test discovery perf analysis of some of the xunit runners. Following are the results for 25k tests discovery on a Win 10 64 bit machine having 8 core 56 GB RAM.

Runner Test discovery time for 25k tests (sec)
Xunit.console 8.23
VSTest.console TPv2 95.17
VisualStudio.xunit runner - TPv2 103.29

Clearly the VisualStudio commandline and IDE runners are lot slower than xunit.console.
PerfView analysis of VSTest.console and VS.xunit adapter revealed following as the top issues:

  1. Communication across Appdomains. Major contributors are:
    ⦁ SourceInformation collection - Filename and linenumber for each test
    ⦁ Xunit to VSTestCase conversion
    ⦁ Xunit discovered tests transfer
    ⦁ Test case object serialization\deserialization

  2. JSON over TCP socket communication, which is a known issue

To ascertain that it is indeed the inter appdomain communication which is taking up maximum time, we modified the xunit code temporarily to force it to work in a single appdomain and found that discovery time for VisualStudio.xunit reduced to 44 sec. However due to some technical limitations, the adapter cannot work in a single appdomain.

Why is xunit.console the fastest ?

Xunit.console is faster than VS xunit and VSTest.console xunit because:
1. Socket communication is not required
2. Source information is not collected
3. Xunit to VSTestCase conversion is not required

2 and 3 saves lot of inter appdomain communication cost for xunit.console

In order to find solution to above problems for VS xunit runners, we compared them with MSTest v2 adapter and found that MSTest takes 30.13 sec to discover 25k tests which is although slower than xunit.console but significantly faster that VS xunit runners.

Why is MSTest v2 faster ?

It is faster than VS xunit and VSTest.console xunit because:

  1. Unlike in xunit, test case object serialization\deserialization is not required
  2. Using following measures, it minimizes communication across appdomains:
    a. Instead of source dll appdomain, source information is collected in default appdomain which has MS.VS.TP.ObjectModel loaded (the DIASession class)
    b. UnitTestElement is serializable and not MarshalByRef. Hence UnitTestElement to VSTestCase conversion doesn't involve cross appdomain marshalbyref cost
    c. Discovered tests get batched and transferred in a single burst across appdomains

Taking inspiration from MSTest V2 adapter design, we found that following changes in xunit adapter can give some perf benefits by reducing the inter appdomain communication cost:

S.No. Issue Resolution Fix in Savings (sec)
a. SourceInformation Get rid of extra appdomain 'xunit.runner.utility' getting created for DiaSessionWrapperHelper class. Instead instantiate it directly xunit.runner.utility 20
b. Inter appdomain cost during Xunit discovered tests transfer Batching xunit.execution, VisualStudio.xunit.runner 11
c. MarshalByRef object cost during xUnitTestCase to VSTestCase conversion Make xunitTestCase object serializable Not yet implemented To be discovered

On making temporary code changes for first issue, we observed 25% improvement:

Issue fixed CLI - Before (sec) CLI - After (sec) CLI - Improvement(%) IDE - Before(sec) IDE - After(sec) IDE - Improvement(%)
(a) 95 71 25.26 103 77 25.24

where, CLI: Command line runner, i.e., vstest.console.exe
IDE: IDE runner, i.e., VisualStudio runner

Fix for other issues will likely involve significant code and design changes and will be taken up later.

We will continue working on these identified issues to improve the xunit adapters performance for VS runners.

@codito
Copy link
Contributor

codito commented Feb 16, 2017

We did more profiling on netcore (to see if there are issues that come up after the appdomain issue is fixed). There are two buckets that need more investigation:

  1. Source information can be turned off for command line runs. We're adding RunConfiguration.DesignMode in runsettings (Make the test run context available for the adapter, to allow a choice between IDE vs Console runs #344) and turning off source information query in xunit (see PR Add support for DesignMode. xunit/visualstudio.xunit#100).
  2. Serialization/Deserialization of XUnitTestCase to VSTestCase. There could be cases (like RunAll + Console) where this may be optimized. Need more investigation.

@codito
Copy link
Contributor

codito commented Mar 18, 2017

Updating the current action plan:

  1. Source Information is a big contributor to slowness. We're prototyping if it's possible to find test source information in the IDE, instead of doing it in the adapter (remove the overhead of parsing PDBs and cross appdomain calls)
  2. App domains also slow down discovery. For desktop projects (in the older test platform), it is not possible to remove appdomains since there are file locks. This is removed in newer test platform (this repo), it supports parallel discovery and the discovery processes are short lived (so no file locking). We're planning to bring in parallel discovery by default (Making Discovery Parallel by default #499)
  3. Auto discovery of test projects is triggered as soon as a solution is opened. Also this operation is not cancellable, it makes things worse; user is blocked on doing any operation on Test Explorer until the auto discovery is complete. We're debating if this feature should be removed, and make discovery operations cancellable

Please share feedback on (3) - if you find auto test discovery feature useful.

@PureKrome
Copy link

re (3) - yep, would love to cancel if it's taking too long. Scenario: I just need to test one test out and fix stuff. Not have to wait etc...

@pvlakshm pvlakshm mentioned this issue Apr 4, 2017
17 tasks
@Shyam-Gupta
Copy link
Member Author

Status:

Fix for SourceInformation issue (a): xunit/xunit#1142 (Review pending)

Partial fix for xUnitTestCase to VSTestCase conversion (c): xunit/visualstudio.xunit#104 (Fix merged)

@snkota
Copy link

snkota commented Nov 7, 2017

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

No branches or pull requests

5 participants