Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initialisation is slow #9

Open
ikkez opened this issue Jan 8, 2020 · 5 comments
Open

Initialisation is slow #9

ikkez opened this issue Jan 8, 2020 · 5 comments

Comments

@ikkez
Copy link

ikkez commented Jan 8, 2020

\Cron::instance(); is slow, because it always calls the binary method which uses an exec call:

f3-cron/lib/cron.php

Lines 54 to 56 in a66f447

function binary($path) {
if (function_exists('exec')) {
exec($path.' -v 2>&1',$out,$ret);

this cant be skipped, even if you'd set a valid path. Some measurements showed a bottleneck here:

Bildschirmfoto 2020-01-08 um 18 29 53

Would love to see some way of avoiding this path check with each and every page load. For now I'm using this workaround:

if ($this->fw->CLI ||
	($this->fw->exists('CRON.web',$web) && $web 
		&& preg_match('/^\/cron\/.*/',$this->fw->PATH)))
	\Cron::instance();

This way it's only called when the cron service is really used. Maybe it could fit into its contruct as well 🤔
thx bro ;)

@xfra35
Copy link
Owner

xfra35 commented Jan 26, 2020

Actually this behaviour was intentional, because the library tries to avoid the situation where asynchronous calls would be silently lost just because the PHP CLI executable could not be found.

Anyway you're right about the performance cost and I realize I've never hit it, precisely because I instanciate the class only when needed:

if (preg_match('/^\/cron/',$f3->PATH)) {
    $f3->DEBUG=2;
    $f3->ONERROR=function($f3){
        KS\Report::instance()->send();
        exit(1);
    };
    $f3->config('apps/cron.ini');
    Cron::instance();
}

So I've added an optional $force parameter to the binary() method. You can use it like this:

$cron->binary('/path/to/php',TRUE);

or in config file:

[CRON]
binary = /path/to/php, TRUE

When this option is enabled, the validation check is bypassed.

@ikkez
Copy link
Author

ikkez commented Jan 30, 2020

well thanks nice of cause that it checks the path automatically. But does it need to do that all the time, even if unsued? What about moving this part

f3-cron/lib/cron.php

Lines 247 to 250 in 161da4d

if (!isset($this->binary))
foreach(['php','php-cli'] as $path) // try to guess the binary name
if ($this->binary($path))
break;

into the execute method?

@xfra35
Copy link
Owner

xfra35 commented Oct 30, 2020

Thanks for the insights. I'll refactor the library when I have a bit more time. I'll see if it's possible to guess the binary "just in time" and/or skip the asynchronicity directly in config file.

@kumy
Copy link

kumy commented Mar 23, 2021

or in config file:

[CRON]
binary = /path/to/php, TRUE

When using this I get an Array to string conversion error:

# php index.php /cron
Array to string conversion
[/var/www/geokrety/vendor/bcosca/fatfree-core/base.php:2347] Base->error()
[/var/www/geokrety/vendor/xfra35/f3-cron/lib/cron.php:56] Base->{closure}()
[/var/www/geokrety/vendor/xfra35/f3-cron/lib/cron.php:243] Cron->binary()
[/var/www/geokrety/vendor/bcosca/fatfree-core/base.php:35] Cron->__construct()
[index.php:20] Prefab::instance()

===================================
ERROR 500 - Internal Server Error

@eazuka
Copy link

eazuka commented Aug 10, 2021

@xfra35 was this ever addressed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants