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
Console runner performance varies wildly depending on environmental characteristics #2217
Comments
Running from the project file is somewhat unusual. That, combined with the fact that VS is involved, makes me suspicious. Can you see what happens if you just run the assembly? |
@CharliePoole That's a good point, however I've just tried it with the same behaviour:
The first run (~7seconds) was with VS closed, the second (~2seconds) was with it open. |
Interesting. As developers, we are almost always running NUnit tests locally with VS running, so those are the times we see. I'll do some experiments. |
That's exactly what we found, it was only on CI that we noticed the run-time difference initially. |
I am unable to duplicate this so I'm marking it for somebody else to try to confirm. When you did your experiments, did you try it with VS open but no solution loaded? I wonder if it could have to do with some assemblies being in memory already. I'm assuming that neither your tests nor your SUT uses VS in any way - i.e. you are not loading the VS object model or anything like that. |
@CharliePoole Yeah it was VS open with no projects loaded at all vs it unopened. The tests are totally in-memory standalone things. There's no relationship to VS in the tests at all. |
I'm running on Windows 10. |
@mintsoft just a guess, but could you repeat the test with another .NET program running, even just a simple console app blocked in a Also, are you only able to reproduce this on your CI machine or are you also reproducing this on your development machines? |
@rprouse I've just tried that, the tests ran in ~9 seconds (that's the slow time) with a console application running in the background. I can reproduce this on dev machines easily enough even with a fresh install of windows. I've been experimenting with other applications being open, interestingly enough it doesn't seem to be limited to Visual Studio. One that was particularly interesting is the ModernUI WPF Example application (https://github.com/firstfloorsoftware/mui/tree/master/1.0/FirstFloor.ModernUI/FirstFloor.ModernUI.App). I compiled it, ran it and found that if it's running when the tests are executed they again run at about 2.5/3 seconds (fast speed). Close it, run the tests again and I'm back to 8 seconds. I am very confused now |
Maybe we are looking at this wrong. Could the problem be in how we report the time? |
@CharliePoole I don't think so, when I'm watching the runs it physically takes longer to watch. I can do a video of the test if you want? |
Looking at the pauses in the output, it looks like it is happening while starting up the remote agent. The actual running of the tests appears to be the same. Could you try running the tests with the |
Sorry, I am wrong, the Run Settings is output at the end of the test run. It still could be related to the remote agent though, so please try the |
@rprouse I've tested that anyway, the |
That's expected since the timing is done entirely within the framework code. |
@CharliePoole I've managed to reproduce it on my Windows 10 pro laptop with basically the same behaviour. 11 Seconds normal, 3 with the ModernUI Application open. So it doesn't look like it's limited to Windows Server 2012 R2 |
I've also reproduced it on my personal desktop machine running Windows 10 Pro, although I had to reboot in order to get the "slow" time, then I got the same behaviour as 2012 R2 and my laptop. |
Hi All, I can reproduce this behaviour. With the ModernUI Application closed, tests take around 10-11 seconds to complete. With it open tests take around 3-4 seconds. My tests can be found here. I have attached the verbose output for the slow and fast runs. Edit* The above results are from my desktop PC running Windows 10 v1703. I have carried out the exact same scenario on my Surface Pro 3 (which is running Windows 8.1). And I get the same behaviour. fast-surface.txt Cheers, |
@singh400 Do you have the output xml files from the runs? |
@mintsoft These are from my desktop PC running Windows 10 v1703:- Slow_TestResult.txt Cheers, |
Thanks, it's interesting. Looking at a few tests there doesn't seem to anything obvious except that the durations are longer on the slower run for all the tests :/ Strange |
@mintsoft I get the same behaviour if I use AnimationExamples instead of ModernUI app. Can you confirm? Cheers, |
Found a better example; AnimationTiming.
Can anyone else confirm? |
@singh400 Yes I can confirm that is exactly what I'm seeing. Half of those examples didn't compile for me, however the AnimationTiming behaves exactly like you stated. This is really odd, it seems like maybe WPF animations are forcing the framework into some sort of high resolution timing mode which is having an effect on the way that NUnit's console runner is scheduling the test execution? |
@mintsoft To be honest, I have no idea. I think what is happening is that the WPFs apps * Edit: Chrome, WPF make changes to the windows timer resolution. Next three posts contain more info. |
Well indeed, this all feels like guesswork, however I was looking at how NUnit schedules the test runs: I can't see anywhere that |
Sounds like this could be related to changes in the windows timer resolution. |
@appel1 So I found this blog post. Using the utility ClockRes I was able to confirm a change in the "Current timer interval" when starting and stopping the animations in AnimationTiming. Which matches the fast and slow behaviour. I also checked when watching a video in Chrome, same changes to the timer interval. |
We changed to Sleep(1) from Sleep(0) because Microsoft issued a document that basically stated that Sleep(0) should never be used! It had nothing to do with the CF build. I think I referenced the document in the original issue. I have not searched for it, but it should be there. I think that Thread.Yield () could be used instead, but there were issues with that as well (besides the fact that it does not exist in CF). |
IIRC |
Is there a reason for the event pump thread to run with highest priority? The commit comment that it fixes a deadlock seems odd to me. Always tricky when you mess with the thread priorities. |
@appel1 Where is this coming from? The event pump thread should not have highest priority at all. It makes blocking calls to unknown event handlers. |
Wow! That has been in there since NUnit 2. Back then, it made a bit more sense. We had control over the event handlers and knew what they did. And using a higher priority made it possible for Sleep(0) to work. |
I'm most likely missing something but I can't observe any issues using nunit-console, nunit-gui or R# if I remove changing the thread priority and sleeps in dequeue even when running the tests in-process. What problem is that code trying to solve? Don't have access to any single core non-ht machines to test on so maybe that is it. public void Enqueue(Event e)
{
// Add to the collection
_queue.Enqueue(e);
// Wake up threads that may have been sleeping
_mreAdd.Set();
}
public Event Dequeue(bool blockWhenEmpty)
{
do
{
// Dequeue our work item
Event e;
if (_queue.TryDequeue (out e))
{
return e;
}
if (!blockWhenEmpty || Interlocked.CompareExchange(ref _stopped, 0, 0) != 0)
return null;
_mreAdd.Wait(-1);
if (Interlocked.CompareExchange(ref _stopped, 0, 0) != 0)
return null;
} while (true);
} |
My inclination is to drop the priority-setting and the Sleep and see what happens over the next month. We can adjust before a release if we have to. |
Are we any closer to choosing a solution (#2233 or #2236)? It has been 18 days since the issue was raised, and 14 days since the regression was identified in commit 9086990. It reads like that the majority of people prefer #2236 in which case I can close #2233 and we can just focus on getting #2236 into the next release. |
The rule is that it requires two committers to approve a merge to master. Neither of these has that yet. Usually, the PR submitter simply makes some changes to get the PR approved, but in this case we appear to have devolved into a more general discussion. 😞 Since we have two PRs, I'll summarize here... #2233 Simply changes the Sleep period. It had one review from @jnm2 who requested changes.
#2236 Drops the Sleep entirely and eliminates the high priority, which is what I asked for in one of my comments. I think that's the easiest solution. However, it goes on to make a bunch of other changes that don't seem related to this problem. This has one review from @rprouse and comments from me.
I'm going to add reviews to each PR. My inclination is to use #2233 right away and hold #2236 for more discussion. @oznetmaster is changing continents right now and will eventually become available again.ging #2233 will make the change available on our MyGet feed for anyone who wants an immediate fix. |
I notice that this still is marked as needing confirmation, although it was confirmed a while back. I'll fix. |
I'd keep it open, since it has the most thorough discussion. It started out as a kind of puzzle and it was only gradually turned into a workable bug by folks on the thread. |
@jskeet has confirmed that #2233 has resolved his issue as reported in nunit/dotnet-test-nunit#109. #2236 and this issue have been quiet, should we close? |
I think that the merge of #2233 was intended to close this. |
I think judging by #2217 (comment) you wanted to keep it open until a decision is made with #2236 |
Ah... too many issues, too many comments. 😄 I'd still say close it. The references from the other issue will still be there. |
That's fine with me |
By reading @indy-singh's great writeup at https://medium.com/@indy_singh/fixing-nunit-slow-test-execution-a67c284355c4, I became aware that JetBrains opted not to use Sleep(0) because it's not guaranteed to yield, as we've discussed in several threads. Instead, they use |
Hiya,
We've just rolled out NUnit 3 for the majority of our build suite, and we've found some interesting (to say the least) performance behaviour. I think this may be a duplicate of: #480
Basically we noticed that tests "are slow on CI"; after eliminating the hardware and installation etc. We've found that tests when executed through NUnit-console are quicker when Visual Studio is open, than when it's closed. This seems insane behaviour, however it is absolutely what we see (absolutely repeatable on multiple machines, including a freshly installed machine):
It happens with every test project we have with comparable performance differences (i.e. 1 minute to 4 minutes on the larger suites). When I say with Visual Studio open, I don't mean the relevant
sln
orcsproj
open, I mean just the application itself.Here the first run was with VS open, the next 2 with it closed and then it open again:
This is running on:
I'm at a loss how to explain or debug this further; happy to provide any info or run any tests you like ??
The text was updated successfully, but these errors were encountered: