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

Setting ServerGC for the test runner #1967

Closed
ivonin opened this issue Mar 22, 2019 · 14 comments
Closed

Setting ServerGC for the test runner #1967

ivonin opened this issue Mar 22, 2019 · 14 comments

Comments

@ivonin
Copy link

ivonin commented Mar 22, 2019

Description

VSTest.console.exe is currently configured to run in workstation GC (also concurrent). There is currently no way to change this behavior other than patching the vstest.console.exe.config file in the VS installation folder. This solution is obviously not pleasant.

We have a big test assembly (11.5k tests) and it makes a big difference: the tests take 9 minutes to run with ServerGC=off and 4.5 minutes with ServerGC=on.

Expected behavior

  1. It's probably not a bad thing to make ServerGC=on by default (MSTest v1 was using server GC, see QTAgent32.exe.Config)
  2. If not, it would be nice to set the behavior via the .runsettings file.
@ShreyasRmsft
Copy link
Member

Consider taking a look at https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/clr-configuration-knobs.md#host-configuration-knobs

If it can be achieved by env vars then you can author a script that sets this assuming you are running in a pipeline

@ShreyasRmsft
Copy link
Member

As for a in-house solution we are open to contributions for making this configurable via runsetttings.

@ivonin
Copy link
Author

ivonin commented Mar 25, 2019

@ShreyasRmsft vstest.console.exe is not a .NET Core app, am I wrong? I'm not sure if the Core CLR configuration knobs are applicable in this case.

Do you know of a similar way of providing GC settings for the desktop .NET Framework app? Nothing comes to my mind.

@ShreyasRmsft
Copy link
Member

Will have to read-up on this. Lemme know if you find anything. I'll also try seeing if there's anything we can do for full .Net

@ivonin
Copy link
Author

ivonin commented Apr 9, 2019

@ShreyasRmsft any update on this?

Could you help me understand which downsides you see in just setting ServerGC=on by default? It seems reasonable considering that the MSTest v1 runner worked that way.

@ShreyasRmsft
Copy link
Member

@ivonin looping in @singhsarab and @mayankbansal018 to help with this

@mayankbansal018
Copy link
Contributor

@ivonin The servergc can be enabled for testhost.exe, testhost.x86.exe, do you think you can raise a PR for this?

@ivonin
Copy link
Author

ivonin commented Apr 10, 2019

@mayankbansal018 I created a PR, see above.

From what I can see vstest.console.exe runs some tests directly, at least in some scenarios, and changing the GC mode for vstest.console.exe fixes the problem. I ended up setting the GC options for all three of the executables.

@Evangelink
Copy link
Member

@nohwnd I see this ticket is still open but related PR was closed and not merge whereas from discussions it seems there was no downside. Is it worth reopening and moving forward with the merge?

@JoshKeegan
Copy link
Contributor

Any reason this never got merged?

While looking at why a large test project runs much faster in Resharper's unit test runner than the Visual Studio or console runners I've found that Resharper is running them with server GC and I believe this to be the cause of the issue. It'd be great to have this be the default for vstest, or at least the ability to opt-in to it.

@ivonin
Copy link
Author

ivonin commented May 18, 2022

@JoshKeegan some checks failed in my PR and to be honest I just didn't have time to properly deal with it back then.

You can try again and see if maybe things have changed since.

@JoshKeegan
Copy link
Contributor

I've opened a new PR, let's see 👍

@JoshKeegan
Copy link
Contributor

@ivonin my PR got merged, so this should be included in a future release & this can be closed 👍

@Evangelink
Copy link
Member

Thanks for the ping @JoshKeegan.

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.

5 participants