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

[NFR] Improve Phalcon\Autoloader speed #13360

Closed
dugwood opened this Issue Apr 29, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@dugwood
Contributor

dugwood commented Apr 29, 2018

Expected and Actual Behavior

Phalcon\Autoloader is slow because the is_file() test is slow. With a lot of files, this can add a serious amount of time, especially if the whole PHP run is small.

See #10472 for instance, and specifically comment: #10472 (comment)

Reproduce script:

$time = -1 * microtime(true);
include __DIR__.'/../library/ModelParameters.php';
include __DIR__.'/../models/Model.php';
include __DIR__.'/../models/Promotion.php';
$files = 3;
foreach (glob(__DIR__.'/../models/*.php') as $file)
{
	$class = 'MyNamespace\\Models\\'.substr(basename($file), 0, -4);
	if ($class === 'MyNamespace\\Models\\Model' || $class === 'MyNamespace\\Models\\Promotion')
	{
		continue;
	}
	include $file;
	$files++;
}
echo ($time + microtime(true)) * 1000, ' - files: '.$files;

=> for 120 files
=> first load: about 120ms
=> next loads: 4-8ms (opcache must play a part here)

$time = -1 * microtime(true);
$loader = new Loader();
$loader->registerNamespaces([
	'MyNamespace\Controllers' => __DIR__.'/controllers/',
	'MyNamespace\Backend\Controllers' => __DIR__.'/../backend/controllers/',
	'MyNamespace\Frontend\Controllers' => __DIR__.'/../frontend/controllers/',
	'MyNamespace\Models' => __DIR__.'/../models/',
	'MyNamespace\Library' => __DIR__.'/../library/',
	'MyNamespace\Plugins' => __DIR__.'/../plugins/',
]);
$loader->register();
echo ($time + microtime(true)) * 1000;

=> takes less than a millisecond as expected

$time = -1 * microtime(true);
$files = 2;
foreach (glob(__DIR__.'/../models/*.php') as $file)
{
	$class = 'MyNamespace\\Models\\'.substr(basename($file), 0, -4);
	class_exists($class);
	$files++;
}
echo ($time + microtime(true)) * 1000, ' - files: '.$files;

=> for 121 files
=> first load: 250ms
=> next loads: 100-150ms (about 1ms per loaded class)

The culprit is the is_file() test before including the class: https://github.com/phalcon/cphalcon/blob/v3.3.2/phalcon/loader.zep#L368

Proposed fix

Add a parameter, either as a new function (setFileChecking) or as a third parameter in registerNamespaces() (see https://github.com/phalcon/cphalcon/blob/v3.3.2/phalcon/loader.zep#L109), for example $this->fileChecking:

  • default => is_file()
  • stream => stream_resolve_include_path() (as it seems to be really faster than is_file(), but implies some issues if the file is removed from the filesystem)
  • none => no file check (you should always have a class file if you load one class, don't you? It's true only if you don't use multiple directories for the same namespace)

Details

  • Phalcon version: 3.3.2
  • PHP Version: 7.0.27
  • Operating System: Linux Debian stable
  • Installation type: installing via package manager
  • Zephir version (if any): (from package) 0.10.7
  • Server: Lighttpd
  • Other related info: none
@dugwood

This comment has been minimized.

Contributor

dugwood commented Apr 29, 2018

@sergeyklay sergeyklay added this to the 3.4.x milestone Apr 29, 2018

@dugwood

This comment has been minimized.

Contributor

dugwood commented Apr 29, 2018

Waiting for PR #13361 to be merged to close this one. PR is working great (and opcache is really important in the end).

@dugwood dugwood closed this Apr 30, 2018

@sergeyklay sergeyklay referenced this issue Aug 2, 2018

Merged

default parameter value matches the type #13448

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment