I'm building a fork of OpenCart that involves some very heavy rewriting of the core functionality to including dedicated themes that allow for overriding the core, similar to the Wordpress theme system.
If a given controller, model or language file exists in the theme, use that, if not, use the core file.
I'm currently autoloading all the system files which works great. But I'm wondering if it would be better to also autoload all my controller, model, and language files as well?
I've noticed that (even in OpenCart) the same file gets searched via the file system, and loaded multiple times. (models and languages in particular)
Would it not be more efficient to autoload all these files then simply instantiate the exiting class, over searching for the files each time then using an include?
Let me give an example ...
My homepage contains the following modules:
Slideshow, Latest Products, Featured Products, Carousel
This along with the header and footer controllers ends up calling:
$this->load->model('catalog/product');
23 times.
That's 23 times that the same exact file is being searched for using the file system. Plus of course all the other models being called.
So I guess the question is really more about overhead ...
Is it more expensive to autoload (and cache) all files regardless of whether they're being called, or search the file system and do includes for each file as it's needed?
I'm sure this is an age old question but I've yet to see a definitive answer.
And BTW either answer is fine, I'd just like to be as efficient as possible in my code.
Well I get Your point but to be honest You should be concerned about this only in the case You can prove that the response times are somehow slower because of this reason. After having such proof (which is relative to the server's hardware, connection speed, whether it is sunny or stormy weather or day or night) You can issue a bug and a patch-fix using the github repository.
Now the model loading performs file_exists
check, then include_once
and afterwards it instantiates a new model object while setting (or replacing) it as some key in the registry. Current code looks like this (system/engine/loader.php
):
public function model($model) {
$file = DIR_APPLICATION . 'model/' . $model . '.php';
$class = 'Model' . preg_replace('/[^a-zA-Z0-9]/', '', $model);
if (file_exists($file)) {
include_once($file);
$this->registry->set('model_' . str_replace('/', '_', $model), new $class($this->registry));
} else {
trigger_error('Error: Could not load model ' . $model . '!');
exit();
}
}
By just slight modification we can improve this code to perform only one file_exists
check, only one include_once
and having only one model's object:
public function model($model) {
$key = 'model_' . str_replace('/', '_', $model);
if (!$this->registry->has($key)) {
$file = DIR_APPLICATION . 'model/' . $model . '.php';
$class = 'Model' . preg_replace('/[^a-zA-Z0-9]/', '', $model);
if (file_exists($file)) {
include_once($file);
$this->registry->set($key, new $class($this->registry));
} else {
trigger_error('Error: Could not load model ' . $model . '!');
exit();
}
}
}
You can now put some time debugging into the original code and to the fixed one and find out whether this brings so great improvement :-)
Edit: Either way I would check how is this handled and done in OpenCart 2.0 (in it's latest status since it is not release-ready so far) and file an issue only if this is done the same way. Otherwise just fix your own installation(s)...