Lazy loading of plugin config and language files

[cmb]

Hi!

Traditionally, CMSimple(_XH) loads all plugin configuration and language files on each request, even if some of these are not used. This is a waste of resources, which can be alleviated by using a byte-code cache, but probably many shared hosters still don't offer one, and we could still save memory by not loading unnecessary files.

Until recently, I thought that it's not possible to change that without breaking almost all plugins, but it seems it can be done by using the ArrayAccess interface. The following patch serves as POC:

Code: Select all

 cmsimple/classes/PluginConfig.php | 56 +++++++++++++++++++++++++++++++++++++++
 cmsimple/cms.php                  | 17 +++---------
 plugins/jquery/jquery.inc.php     |  2 +-
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/cmsimple/classes/PluginConfig.php b/cmsimple/classes/PluginConfig.php
new file mode 100644
index 0000000..0988048
--- /dev/null
+++ b/cmsimple/classes/PluginConfig.php
@@ -0,0 +1,56 @@
+<?php
+
+class XH_PluginConfig implements ArrayAccess
+{
+    private $language;
+
+    private $configs = array();
+
+    public function __construct($language = false)
+    {
+        $this->language = $language;
+    }
+
+    public function offsetExists($offset)
+    {
+        if (!isset($this->configs[$offset])) {
+            $this->loadConfig($offset);
+        }
+        return isset($configs[$offset]);
+    }
+
+    public function offsetGet($offset)
+    {
+        if (!isset($this->configs[$offset])) {
+            $this->loadConfig($offset);
+        }
+        return $this->configs[$offset];
+    }
+
+    public function offsetSet($offset, $value)
+    {
+        if (!isset($this->configs[$offset])) {
+            $this->loadConfig($offset);
+        }
+        $this->configs[$offset] = $value;
+    }
+
+    public function offsetUnset($offset)
+    {
+        if (!isset($this->configs[$offset])) {
+            $this->loadConfig($offset);
+        }
+        unset($this->configs[$offset]);
+    }
+
+    private function loadConfig($pluginname)
+    {
+        global $pth;
+    
+        pluginFiles($pluginname);
+        if ($this->language) {
+            XH_createLanguageFile($pth['file']['plugin_language']);
+        }
+        $this->configs += XH_readConfiguration(true, $this->language);
+    }
+}
diff --git a/cmsimple/cms.php b/cmsimple/cms.php
index 54f6762..e104aa5 100644
--- a/cmsimple/cms.php
+++ b/cmsimple/cms.php
@@ -275,6 +275,7 @@ require_once $pth['folder']['classes'] . 'PasswordHash.php';
 require_once $pth['folder']['classes'] . 'PageDataRouter.php';
 require_once $pth['folder']['classes'] . 'PageDataModel.php';
 require_once $pth['folder']['classes'] . 'PageDataView.php';
+require_once $pth['folder']['classes'] . 'PluginConfig.php';
 require_once $pth['folder']['classes'] . 'PluginMenu.php';
 require_once $pth['folder']['plugins'] . 'utf8/utf8.php';
 require_once UTF8 . '/ucfirst.php';
@@ -1110,7 +1111,7 @@ $pd_current = $pd_router->find_page($pd_s);
  *
  * @see $cf
  */
-$plugin_cf = array();
+$plugin_cf = new XH_PluginConfig();
 
 /**
  * The localization of the plugins.
@@ -1123,19 +1124,7 @@ $plugin_cf = array();
  *
  * @see $tx
  */
-$plugin_tx = array();
-
-/*
- * Include config and language files of all plugins.
- */
-foreach (XH_plugins() as $plugin) {
-    pluginFiles($plugin);
-    $temp = XH_readConfiguration(true, false);
-    $plugin_cf += $temp;
-    XH_createLanguageFile($pth['file']['plugin_language']);
-    $temp = XH_readConfiguration(true, true);
-    $plugin_tx += $temp;
-}
+$plugin_tx = new XH_PluginConfig(true);
 
 /*
  * Add LINK to combined plugin stylesheet.
diff --git a/plugins/jquery/jquery.inc.php b/plugins/jquery/jquery.inc.php
index 805e800..32d766c 100644
--- a/plugins/jquery/jquery.inc.php
+++ b/plugins/jquery/jquery.inc.php
@@ -27,7 +27,7 @@ if (!defined('CMSIMPLE_XH_VERSION')) {
 global $hjs, $plugin_cf, $pth;
 
 //load plugin-configuration for xh < 1.6
-require($pth['folder']['plugins'] . 'jquery/config/config.php');
+// require($pth['folder']['plugins'] . 'jquery/config/config.php');
 
 function include_jQuery($path = '') {
     global $pth, $plugin_cf, $hjs;

So far I did only some quick testing with the standard distribution of CMSimple_XH 1.6.7, and have not been able to find any issues. I assume that a few plugins might break after these changes, but maybe even this could be avoided by implementing also Countable and/or Iterator.

[manu]

sounds very interesting. Could this be done also with the plugin.css files?
regards
manu

[cmb]

Could this be done also with the plugin.css files?

I don't think it's possible without changing the API (i.e. plugins would have to call a function to register their desire to include their stylesheet in a particular request). On the other hand, I don't think there's a strong need with regard to plugins.css. Usually, the file is supposed to be downloaded once; to avoid further requests, it's possible to set up respective caching headers via the Web server.

[cmb]

I've just noticed that I had already suggested to address this issue in an earlier thread. Considering BC, this solution would be preferable, though, at least for XH 1.x.

[manu]

I've just noticed that I had already suggested to address this issue in an earlier thread. Considering BC, this solution would be preferable, though, at least for XH 1.x.

But this is on the 2.0 roadmap, right? Please let lift the minimum requirements for 2.0 at least php 5.0.

[cmb]

I've just noticed that I had already suggested to address this issue in an earlier thread. Considering BC, this solution would be preferable, though, at least for XH 1.x.

But this is on the 2.0 roadmap, right?

Ah, there's probably a misunderstanding. The suggestion of this very thread is already on the it's on the 1.7 roadmap, and it is preferable over the suggestion in the other thread for BC reasons, because most (maybe all) plugins would work as before.

Please let lift the minimum requirements for 2.0 at least php 5.0.

PHP 5.0?

Note that we already have decided to require at least PHP 5.3 for XH 1.7. Considering the long delay I wouldn't be opposed to require PHP 5.4 for XH 1.7, and I'm certainly in favor of lifting the PHP requirements further for future versions. We (me) shouldn't make the same mistake again, trying to support totally obsolete PHP versions.

If you meant PHP 7: I'm strongly against dropping PHP 5.6 support before its EOL, which is on 31 Dec 2018, and that's pretty far away. Let's see what happens until then. (And yes, I'm eager to use some of the new PHP 7 features, but I'm afraid that even generators and other great stuff introduced late in PHP 5 are too demanding for our user-base.)

[cmb]

I have implemented this in a feature branch, and did a simple measurement compared to master: request the start page of a CMSimple_XH standard installation and compare the number of times XH_includeVar() is called. While it is called 16 times with master (i.e. 16 configuration or language files are included), it is called only 5 times with the feature branch! \o/

[manu]

This is an extract of Holger's hi_fancybox:

Code: Select all

while (list( $key, $val ) = each($plugin_cf['hi_fancybox'])) {

It seems that the access to our mentioned arrays is a kind of delicate. It seems that each() doesn't work as expected. It ends up looping with a bunch of "Indirect modification of overloaded element of XH\PluginConfig has no effect" Messages. I suppose the pointer is not moved forward.
Do we need certain explanation how to access these arrays?

[cmb]

It seems that each() doesn't work as expected.

each() is deprecated as of PHP 7.2.0, anyway, so I don't think we have to make adjustments to cater to such loops. Instead, each-loops should be "fixed" in the plugins that are using them, right away; particularly note:

The each() function is inferior to foreach in pretty much every imaginable way, including being more than 10 times slower.

Frankly, I like to add:

The each() function is inferior to foreach in pretty much every imaginable way, including being more than 10 times less readable.

IMHO, each() should have been already deprecated as of PHP 5.0.0, but on the other hand, I was rather delighted to see that it has been deprecated as of PHP 7.2.0 (especially considering, that it was the only really controversal deprecation in PHP 7.2.0 so far).

It ends up looping with a bunch of "Indirect modification of overloaded element of XH\PluginConfig has no effect" Messages

I think that happens if any attempt to modify $plugin_cf[…] is made. Actually, that shouldn't be done, since $plugin_cf (and $plugin_tx) are documented to be read-only at least as of CMSimple_XH 1.6.

[Holger]

This is an extract of Holger's hi_fancybox:

Code: Select all

while (list( $key, $val ) = each($plugin_cf['hi_fancybox'])) {

It seems that the access to our mentioned arrays is a kind of delicate. It seems that each() doesn't work as expected. It ends up looping with a bunch of "Indirect modification of overloaded element of XH\PluginConfig has no effect" Messages. I suppose the pointer is not moved forward.
Do we need certain explanation how to access these arrays?

Hmm, that's already fixed in the latest V4.1:

Code: Select all

//Read settings for FancyBox from plugin_cf
    $fcbSettings = '';
    foreach ($plugin_cf['hi_fancybox'] as $key => $val) {
    ...

You should update