Skip to content

Caches failed service probes - #22944

Merged
arnaud-lb merged 10 commits into
php:masterfrom
NickSdot:perf/cache-test-service-probes
Aug 6, 2026
Merged

Caches failed service probes#22944
arnaud-lb merged 10 commits into
php:masterfrom
NickSdot:perf/cache-test-service-probes

Conversation

@NickSdot

@NickSdotNickSdot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Extracted from #22917.

Prevents unavailable optional test services from slowing the suite through repeated connection timeouts. PDO and SNMP availability probes cache the first failure for the duration of one test run, so later SKIPIF checks return immediately.

Only failures are cached; successful probes and test connections are unchanged. PDOs one second timeout applies only to the default availability probe. The caches are private, run-scoped and removed afterwards. A service becoming available mid run is detected on the next test-suite run.

Result

Values in seconds; measured on macOS.

modemasterthis PRsavedspeedup
-j1285.2468.991276.25531,72×
-j10221.1444.560216.58448,49×

Reproduction:

sapi/cli/php run-tests.php -q -j1 ext/pdo ext/pdo_dblib ext/snmp
sapi/cli/php run-tests.php -q -j10 ext/pdo ext/pdo_dblib ext/snmp

Result (after #22944 (comment) )

Added caching for ext/ldap, ext/pgsql, ext/mysqli, ext/pdo_mysql, ext/odbc.

modemasterthis PRsavedspeedup
-j1313.74035.177278.5638,91x
-j10241.47413.781227.69317.52x

Reproduction:

sapi/cli/php run-tests.php -q -j1 ext/pdo ext/pdo_dblib ext/snmp ext/ldap ext/pgsql ext/mysqli ext/pdo_mysql ext/odbc
sapi/cli/php run-tests.php -q -j10 ext/pdo ext/pdo_dblib ext/snmp ext/ldap ext/pgsql ext/mysqli ext/pdo_mysql ext/odbc

@TimWollaTimWolla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without looking too deeply into it: There is already a SkipCache in the test runner, why doesn't it work?

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Without looking too deeply into it: There is already a SkipCache in the test runner, why doesn't it work?

SkipCache is not actually related. This here actively caches expensive failures across parallel workers so that same following calls do not run into the same timeouts over and over again.

@arnaud-lb

Copy link
Copy Markdown
Member

I believe it has the same purpose, but it's less efficient since it doesn't share its state between workers. However it's more general and avoids introducing complexity into individual tests, so if we could improve it, this would be better.

We could update SkipCache so that it shares its state between workers. E.g. store results in individual files /tmp/run-tests-skipif-{pid}/{key}/{code hash} instead of this->skips[$key][$code].

@NickSdot

Copy link
Copy Markdown
ContributorAuthor

Went back and forth on the comments... Maybe my brain is not braining, but I still don't see how SkipCache is related or could do what is proposed here even if we share it between workers.

SkipCache caches SKIPIF by its code/key. What is proposed here caches failed service probes between different SKIPIF. Changing SkipCache to file based would share some same SKIPIF between workers, but different SKIPIF running the same probes would still not share anything!? The -j1 result proves that, though. All tests already use the same in-memory SkipCache yet we get a huge speedup. Sharing between workers cannot produce that gain.

Agreed on the in-test complexity. But we will not fully get around having something that tells when different tests run the same probe? I pushed an update that makes this more general (ext/ldap, ext/pgsql, ext/mysqli, ext/pdo_mysql, ext/odbc now included), keeps complexity in helpers, reduced the diff from 42 to 14 files, and is now 31,72x and 48,49x instead of 12.66× and 17.34× (on the initial reproduction).


Asides:

  • sharing SkipCache between workers could be useful on its own. Something we could explore in a separate PR.
  • the CI label can be removed; it was added when I excluded tests for debugging.

@NickSdotNickSdot changed the title Cache failed PDO and SNMP service probesCaches failed service probesAug 3, 2026
@NickSdot
NickSdotforce-pushed the perf/cache-test-service-probes branch from 300a315 to 2dd0865CompareAugust 3, 2026 17:11

@arnaud-lbarnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, you are right!

I did a first review, will continue after your feedback.

Comment threadtests/probe_cache.inc Outdated
Comment threadext/pdo_mysql/tests/inc/mysql_pdo_test.inc Outdated
Comment threadtests/probe_cache.inc Outdated

@arnaud-lbarnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Only two suggestion comments: let me know if you want to apply them. Otherwise I will merge.

Comment threadext/pdo_mysql/tests/inc/mysql_pdo_test.inc
Comment threadtests/probe_cache.inc
@NickSdot

NickSdot commented Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

@arnaud-lb I consider this done. Would appreciate if you would prioritise #22995 before any others to give me a faster CI feedback loop.

@arnaud-lb
arnaud-lb marked this pull request as ready for review August 6, 2026 09:41
@arnaud-lb
arnaud-lb merged commit 5d58876 into php:masterAug 6, 2026
18 checks passed
@arnaud-lb

Copy link
Copy Markdown
Member

Thank you!

@NickSdot
NickSdot deleted the perf/cache-test-service-probes branch August 10, 2026 07:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@NickSdot@arnaud-lb@TimWolla