POC run-tests.php: avoid SKIPIF evaluation in subprocesses

[staabm]

as discussed in #16623 this PR implements a proof of concept.

created a rough prototype which main focus is to give an indication how much faster the test-suite can get by avoiding subprocesses running php run-tests.php ext/intl/tests.

• 77 SKIPIF conditions were evaluated in the main process instead of a subprocess.
• scoped on ext/intl/tests only we can see ~2 seconds saved when running on macos.
• using the side-effects-detector libs on other projects showed the impact on windows usually is way bigger.

WITH THIS PR

=====================================================================
Number of tests :   519               411
Tests skipped   :   108 ( 20.8%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :    47 (  9.1%) ( 11.4%)
Expected fail   :     1 (  0.2%) (  0.2%)
Tests passed    :   363 ( 69.9%) ( 88.3%)
---------------------------------------------------------------------
Time taken      : 15.005 seconds
=====================================================================

BEFORE

=====================================================================
Number of tests :   519               411
Tests skipped   :   108 ( 20.8%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :    42 (  8.1%) ( 10.2%)
Expected fail   :     1 (  0.2%) (  0.2%)
Tests passed    :   368 ( 70.9%) ( 89.5%)
---------------------------------------------------------------------
Time taken      : 17.556 seconds
=====================================================================

---

Testing on windows with php 8.2.12 (NTS Visual C++ 2019 x64)

WITH THIS PR

=====================================================================
Number of tests :   519               410
Tests skipped   :   109 ( 21.0%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :    58 ( 11.2%) ( 14.1%)
Expected fail   :     1 (  0.2%) (  0.2%)
Tests passed    :   351 ( 67.6%) ( 85.6%)
---------------------------------------------------------------------
Time taken      : 31.208 seconds
=====================================================================

BEFORE

=====================================================================
Number of tests :   522               410
Tests skipped   :   112 ( 21.5%) --------
Tests warned    :     0 (  0.0%) (  0.0%)
Tests failed    :    53 ( 10.2%) ( 12.9%)
Expected fail   :     1 (  0.2%) (  0.2%)
Tests passed    :   356 ( 68.2%) ( 86.8%)
---------------------------------------------------------------------
Time taken      : 35.888 seconds
====================================================================

---

next steps

• is it worth further exploring?
• see how the impact is on the full test-suite
• estimate how much tests could potentially benefit from this optimization
  update: searching php-src for die\s*\(\s*.skip yields ~ 3200 results (of which not all can be inlined, because of side-effects in skipif)
• look into why "tests failed" is different between the 2 runs
• look into side-effects-detector whether it makes sense to support die("skip xy") cases, so tests don't need manual adjusting

[iluuu1994]

Thank you for your proposal! I have a few concerns.

• I was never an extension developer, but FWIU run-tests.php is deployed as a single file to some environments to run tests. Making it dependent on another file (functionMetadata.php in this case) would break these environments.
• run-tests.php is already large and complex. SideEffectsDetector doesn't look trivial. As it works on a token stream, I'm also assuming it's not entirely accurate, but more of a best guess?
• Sometimes, SKIPIF sections die. Either they contain a mistake themselves, they segfault, timeout, etc. This will take the worker down. That said, I'm assuming such tests will be detected as having side-effects, so it's likely not an issue.
• I'm not too fond of changing die to echo. This will lead to an incredible amount of code churn. That said, it's annoying but not a blocker in itself.

I'm not sure if this is a better approach, but an alternative idea might be to instead:

• Add new directives/annotations that indicate SKIPIF/CLEAN/etc. may be execute in the main process.
• Add a separate script that detects side-effects in existing tests and adds the annotation where applicable.
• That would avoid running side-effect detection at runtime, which seems reasonable if the main aim is to improve performance.
• This will lead to a similar amount of code churn as mentioned above, so it might not be preferable either.

All in all, I'm somewhat skeptical whether this is something we need. With the latter approach, it may be easier to test how much of a difference it actually makes across all tests. If others have anything to add, please do.

[iluuu1994]

$php is not necessarily the same as the currently running PHP binary. This should be handled accordingly.

[cmb69]

@iluuu1994, it just occured to me that it might be possible to extend CLI to accept multiple scripts which are run as separate requests. I have no idea how much work would be needed, if that would work at all, and how badly this might be misused by others, though.

[iluuu1994]

@cmb69 I don't understand how multiple files as inputs to the CLI SAPI would with this issue. Can you elaborate?

[cmb69]

My idea was, that you would basically pass in:

• the skipif section
• the file section
• the cleanup section

Each of these would run as separate request consecutively (so wouldn't influence the state of the PHP interpreter, so side effects wouldn't matter, so no need to detect these upfront). We would avoid multiple PHP processes to start up for a single test.

Now I see that this wouldn't work, since if skipif reports to skip, the others would be executed anyway. Unless we would bake in the relevant logic from run-tests.php, what seems to be a bad idea. We might consider to introduce a new SAPI for this purpose, though. But that wouldn't test the real CLI SAPI, so probably a bad idea anyway.