Run test subprocesses without shell - #22957
Conversation
arnaud-lb
left a comment
There was a problem hiding this comment.
Representing commands as arrays is a good idea in general, and the speed up from the reduced overhead is nice.
However, can we always do that, and always skip the shell? Otherwise we are increasing complexity.
Did as good as possible. Naturally this widened scope and hence diff churn. Either it's just fine as is (diff churn wise) or we cherry the last commit in yet another PR.
We still need |
arnaud-lb
left a comment
There was a problem hiding this comment.
This looks great!
Just two minor comments. I will merge once these are resolved.
Feel free to add a NEWS entry
|
c36e664 reverts some of the previously commit changes because of the CI failure. No other idea for that unless, as said earlier, we would parse Edit: |
2a8b3ef to
3cae725
Compare
3cae725 to
074aaba
Compare
|
Got it back passing on Windows; CI is green. Aside: I had edited the test matrix to only run Windows tests while debugging, that added the CI label and is probably why Tim was requested as reviewer. The edit that triggered it is already reverted. |
Extracted from #22917.
Runs eligible test, SKIPIF, and CLEAN subprocesses through structured proc_open() commands instead of a shell. This avoids shell parsing and Windows cmd.exe launch overhead; tests requiring shell semantics are unchanged.
This is actually more like a prerequisite for the rest, but has performance gain as well.
Result
Values in seconds; measured on macOS.
-j1-j10Reproduction:
Run on
masterand this PR: