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

[EXPERIMENTAL] Skip slow tests on Appveyor #3789

Closed
wants to merge 2 commits into
base: master
from

Conversation

7 participants
@cmb69
Copy link
Contributor

cmb69 commented Feb 2, 2019

Our Appveyor CI times out very often since quite a while. Let's see
whether we can trim down the build time considerably by skipping slow
tests.

Skip slow tests on Appveyor
Our Appveyor CI times out very often since quite a while.  Let's see
whether we can trim down the build time considerably by skipping slow
tests.
cd "%APPVEYOR_BUILD_FOLDER%"
nmake test TESTS="%OPCACHE_OPTS% -q --offline --show-diff --show-slow 1000 --set-timeout 120 -g FAIL,XFAIL,BORK,WARN,LEAK,SKIP --temp-source c:\tests_tmp --temp-target c:\tests_tmp"

This comment has been minimized.

@nikic

nikic Feb 2, 2019

Member

Why is the --show-slow dropped here?

It might be interesting to do a run with --show-slow 0 to get a full list of test timings, including the "fast" ones (which are probably comparatively slow on AppVeyor).

This comment has been minimized.

@cmb69

cmb69 Feb 2, 2019

Author Contributor

Oh, I've mistankenly assumed that --show-slow is only about slow tests.

Anyhow, it seems skipping the slow tests saves quite some time.

@cmb69

This comment has been minimized.

Copy link
Contributor Author

cmb69 commented Feb 2, 2019

Well, maybe we should also review some very slow tests (20+ secs for which purpose).

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 2, 2019

Based on the full log of test timings, it looks like our baseline execution time for tests on AppVeyor is about 0.05s. That seems very slow to me. This seems to indicate that PHP has a slow startup and/or shutdown on Windows. Maybe we can do something about that?

I remember an issue I had at some point on Linux, where OpenSSL decided to do some kind of FIPS verification on startup -- that took just a couple of ms, but was enough to really blow up test times, because we spawn so many processes. Maybe something similar could be the cause here...

@krakjoe

This comment has been minimized.

Copy link
Member

krakjoe commented Feb 3, 2019

@weltling any ideas with regard to this slow startup/shutdown thing ?

@staabm

This comment has been minimized.

Copy link
Contributor

staabm commented Feb 5, 2019

nikic deleted a lot of tests with 8bc663f

is this still required?

@cmb69 cmb69 changed the title Skip slow tests on Appveyor [EXPERIMENTAL] Skip slow tests on Appveyor Feb 5, 2019

@cmb69

This comment has been minimized.

Copy link
Contributor Author

cmb69 commented Feb 5, 2019

This was mostly meant as an experiment. Let's wait if we find out more regarding the slow startup time.

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 5, 2019

@staabm I think all of those removed tests were pretty quick, so even on Windows this probably only shaved a minute or two off the build.

@weltling

This comment has been minimized.

Copy link
Contributor

weltling commented Feb 6, 2019

The most of the bottleneck on Windows is the filesystem. One might find, that some code, where the measurement is only done to the time when all the files/modules are loaded and the code that doesn't touch filesystem is not worse than on other platforms. Specific factors for that are security and the complex ACLs, other programs like antivirus software can inject themselves into some calls, etc. There is no good way to improve this except having a fast hardware and do some system tuning.

My guess on the AppVeyor performance is, that the jobs might run on a lower priority while the tests carve a lot of resources and are numerous. Also every version adds more and more tests, which increases the job runtime. In the todays run - 7.3.2 has 16344 tests, 7.2.15 has 15988 tests. Clear, that not all of them run, but they still need to be read in, processed for at least skip sections, etc. The slowdown seems improportional, but it's of course also about the tests themselves. It might make sense to talk to AppVeyor support first and go from there.

Thanks.

@weltling

This comment has been minimized.

Copy link
Contributor

weltling commented Feb 8, 2019

I've filed a ticket on the AppVeyor support https://help.appveyor.com/discussions/problems/20189-performance-issues-in-php-core-tests .

Thanks.

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 12, 2019

It looks like the timeout issue is fixed now. It would of course still be good to reduce the long build times, but I guess we won't have to skip slow tests.

@weltling Do you know whether parallel test running would help on Windows?

@nikic nikic closed this Feb 12, 2019

@KalleZ

This comment has been minimized.

Copy link
Contributor

KalleZ commented Feb 12, 2019

@nikic It does big time when I was testing @hikari-no-yume's PR here: #2822 it was insanely fast and really worth while for Windows

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 12, 2019

Okay, maybe I'll take a look at finalizing that PR then.

@hikari-no-yume

This comment has been minimized.

Copy link
Contributor

hikari-no-yume commented Feb 12, 2019

That patch is hell to rebase and I basically need to redo it, but I could maybe have a crack at it today or tomorrow.

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 12, 2019

@hikari-no-yume Please feel free to directly commit the first commit (to PHP-7.4). That's probably a big and unnecessary source of conflicts, and makes reviewing harder as well.

@weltling

This comment has been minimized.

Copy link
Contributor

weltling commented Feb 14, 2019

@nikic Parallel runs would sure accelerate the test suite, likely worth while to pursue. Regarding the Azure Pipelines - if we have some volunteers, probably should take a look at it (especially as it also has Mac).

Thanks.

@cmb69 cmb69 deleted the cmb69:appveyor-skip-slow branch Feb 15, 2019

php-pulls pushed a commit that referenced this pull request Feb 16, 2019

run-tests.php: move all top-level code into a function
Prerequisite for parallelised testing:
#2822

Quoth Nikita @
#3789 (comment):

> @hikari-no-yume Please feel free to directly commit the first commit
> (to PHP-7.4). That's probably a big and unnecessary source of
> conflicts, and makes reviewing harder as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment