Run test subprocesses without shell - #22957
Conversation
arnaud-lb
left a comment
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.
| $pass_options .= ' -n'; | ||
| $pass_options_args[] = '-n'; |
There was a problem hiding this comment.
It appears that both variables are equivalent except in representation. I suggest to remove $pass_options and to build that representation from $pass_options_args when needed: implode(' ', array_map(escapeshellarg(...), $pass_option_args)).
| $extra_options = ''; | ||
| $extraOptionArgs = []; |
There was a problem hiding this comment.
Same here (also, variable name should use snake case for consistency with $extra_options)
| return $data; | ||
| } | ||
|
|
||
| function can_run_with_structured_test_command(TestFile $test): bool |
There was a problem hiding this comment.
It is unclear to me why these tests can not use structured commands. Is it because it's not implemented, or because of a technical limitation? We should try to implement if possible.
2 participants
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: