[Runtime] Automatically use FrankenPHP runner when its worker mode is detected #60503
Conversation
41b4f54 to
5a1bd6e
Compare
| * | ||
| * @author Kévin Dunglas <kevin@dunglas.dev> | ||
| */ | ||
| class FrankenPhpRuntime extends SymfonyRuntime |
There was a problem hiding this comment.
no need for a separate runtime if we do the merge
providing FrankenPHP support should work without configuring any runtime IMHO
There was a problem hiding this comment.
Indeed, I'm having a look to integrate it in SymfonyRuntime
There was a problem hiding this comment.
Both runtimes are now merged
|
|
||
| $handler = function () use ($server, &$sfRequest, &$sfResponse, $xdebugConnectToClient): void { | ||
| // Connect to the Xdebug client if it's available | ||
| if ($xdebugConnectToClient) { |
There was a problem hiding this comment.
should we use if (\extension_loaded('xdebug') && \function_exists('xdebug_connect_to_client')) inline instead ?
OPCache will resolve those \extension_loaded will be resolved at compile time:
- when the extension is indeed loaded
- when the extension is not loaded and
enable_dlisfalse(which is probably the case in frankenphp)
function_exists is also resolved at compile time when the function is defined built-in (a false value is not cached as it could exist in userland).
This might have even less overhead than caching the function_exists check in a variable and checking the variable each time. The only thing that would not work anymore is polyfilling xdebug_connect_to_client in userland, but I don't think this can be polyfilled anyway (userland cannot emulate connecting the Xdebug debugger)
There was a problem hiding this comment.
Works for me, I updated accordingly.
5a1bd6e to
1be5a66
Compare
| @@ -142,13 +145,18 @@ public function __construct(array $options = []) | |||
| } | |||
|
|
|||
| $options['error_handler'] ??= SymfonyErrorHandler::class; | |||
| $options['frankenphp_loop_max'] = (int) ($options['frankenphp_loop_max'] ?? $_SERVER['FRANKENPHP_LOOP_MAX'] ?? $_ENV['FRANKENPHP_LOOP_MAX'] ?? 500); | |||
There was a problem hiding this comment.
Shouldn't we find a more explicit name? More generic also?
There was a problem hiding this comment.
Suggestion: worker_loop_max as it's specific to worker mode.
|
|
||
| parent::__construct($options); | ||
| } | ||
|
|
||
| public function getRunner(?object $application): RunnerInterface | ||
| { | ||
| if ($application instanceof HttpKernelInterface) { | ||
| if ($_SERVER['FRANKENPHP_WORKER'] ?? false) { |
There was a problem hiding this comment.
We should check $_ENV also, and use filter_var to read the boolean
| @@ -88,6 +90,7 @@ class SymfonyRuntime extends GenericRuntime | |||
| * debug_var_name?: string, | |||
| * dotenv_overload?: ?bool, | |||
| * dotenv_extra_paths?: ?string[], | |||
| * frankenphp_loop_max?: int, | |||
There was a problem hiding this comment.
We might want to improve a bit the phpdoc of the class in relation to this?
Currently, to use FrankenPHP worker mode, you have to install the
runtime/frankenphp-symfonypackage and set theAPP_RUNTIMEenv var to the correct FQCN.@dunglas and I would like to move the runtime and the runner classes to the Symfony core now that The PHP Foundation officially supports FrakenPHP.
This PR adds FrankenPHP worker mode detection and registers the correct runner to use it. This specific runner is only used when using the worker mode. When using FrankenPHP in classic mode,
HttpKernelRunneris still used.The DX would be greatly improved, given that it works out of the box with this PR.
Classes and tests are coming from this repo with a few tweaks to stick to Symfony CS.