f3-cron icon indicating copy to clipboard operation
f3-cron copied to clipboard

Initialisation is slow

Open ikkez opened this issue 5 years ago • 5 comments

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

https://github.com/xfra35/f3-cron/blob/a66f447e004f55a3d640adb4fcf39ac5628b2939/lib/cron.php#L54-L56

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 ;)

ikkez avatar Jan 08 '20 17:01 ikkez

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.

xfra35 avatar Jan 26 '20 09:01 xfra35

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

https://github.com/xfra35/f3-cron/blob/161da4d136f0a2a27c5f99a99ee738b9cafbd19b/lib/cron.php#L247-L250

into the execute method?

ikkez avatar Jan 30 '20 22:01 ikkez

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.

xfra35 avatar Oct 30 '20 11:10 xfra35

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

kumy avatar Mar 23 '21 21:03 kumy

@xfra35 was this ever addressed?

eazuka avatar Aug 10 '21 19:08 eazuka