run-tests: add skip cache to improve performance #6681
Conversation
MaxSem
changed the title
|
|
||
| if (isset($this->skips[$key][$code])) { | ||
| $this->hits++; | ||
| return $this->skips[$key][$code]; |
There was a problem hiding this comment.
If keepFile is enabled and we have a cache hit, then we'll not keep the file (by dint of not creating it in the first place).
Might make sense to simply not cache if keepFile is enabled?
There was a problem hiding this comment.
Just made it save the file on early exit.
Currently every --SKIPIF-- section in every test file results in 1 extra execution of PHP, every --EXTENSIONS-- section - in 2 executions. This is quite wasteful, as skip checking code is extremely repetitive and extensions are fixed for every binary/ini/command parameters combination. This patch adds caching to all such checks. On my machine, the gains are quite noticeable: 36s instead of 43s with -j16, 292s instead of 337s without concurrency. Cache stats are 3780 hits, 1247 misses in the latter case. In the future, tests could be adjusted to have more uniform skip checks to improve performance even more.
|
There's a failure of ext/mysqli/tests/mysqli_pam_sha256_public_key_option_invalid.phpt on some jobs that I think is related to this change (https://dev.azure.com/phpazuredevops/PHP/_build/results?buildId=14943&view=ms.vss-test-web.build-test-results-tab&runId=340192&paneView=debug&resultId=107434). |
|
Looking. |
|
@nikic, since tests are running faster now, the race condition with various mysqli tests recreating the same table is more likely to occur. Also, this extension's tests frequently mix skip checks with database setup for tests, making it possible to run a test without setting up tables for it, but that's not a cause for that particular failure because the test has unique skipif code. I'll try to address these issues soon. |
|
@MaxSem mysqli tests don't run in parallel -- there shouldn't be race conditions there. |
|
Hmm, how 'bout we put a temporary plug to avoid caching mysqli tests to see if it fixes nightlies? Meanwhile, these tests need fixing anyway to even work with recent mysql... |
See php#6681 for discussion.
Currently every --SKIPIF-- section in every test file results in 1
extra execution of PHP, every --EXTENSIONS-- section - in 2 executions.
This is quite wasteful, as skip checking code is extremely repetitive
and extensions are fixed for every binary/ini/command parameters
combination.
This patch adds caching to all such checks.
On my machine, the gains are quite noticeable: 36s instead of 43s
with -j16, 292s instead of 337s without concurrency. Cache stats are
3780 hits, 1247 misses in the latter case. In the future, tests could
be adjusted to have more uniform skip checks to improve performance even
more.