Optimization for elgg_get_plugin_from_id

[srokap]

I've noticed that when using multiple plugin settings, good piece of a second is taken by multiple elgg_get_plugin_from_id calls. It seems it's written not efficient to say the least.

We've made some optimization for this function, we fetch all of the plugins data by single call and instantiate objects in lazy manner. Such approach saved us about 20-30 DB calls, and sum of the runtime dropped from 200-300ms to around 10ms.

Patch in attachment.

[srokap]

patch for file engine/lib/plugins.php

[srokap]

P.S. Original code version should be 1.8.2 but I can't find it in the list, is it normal or I'm missing something?

[ewinslow]

Can you submit this as a pull request on github? This definitely sounds like a nice optimization.

[srokap]

I've opened pull request. It seems that my fork went quite messy. Final difference changes only one file. ​https://github.com/Elgg/Elgg/pull/209

[cash]

Too much noise in the pull request. Here is the relevant code that I could find:

function elgg_get_plugin_from_id($plugin_id) {
	$plugin_id = sanitize_string($plugin_id);
	$db_prefix = get_config('dbprefix');

	static $plugins;
	static $pluginsRaw;
	
	if(!is_array($plugins)) {
		$options = array(
			'type' => 'object',
			'subtype' => 'plugin',
			'joins' => array("JOIN {$db_prefix}objects_entity oe on oe.guid = e.guid"),
			'callback' => '',//fetch raw data
			'selects' => array('oe.title'),
			'limit' => 0,
		);
		$pluginsRet = elgg_get_entities($options);
		$plugins = array();
		$pluginsRaw = array();
		foreach($pluginsRet as $row) {
			$pluginsRaw[$row->title] = $row;
		}
	}
	
	if(isset($plugins[$plugin_id])) {
		return $plugins[$plugin_id];
	}
	else if(isset($pluginsRaw[$plugin_id])) {//make lazy conversion
		$plugins[$plugin_id] = entity_row_to_elggstar($pluginsRaw[$plugin_id]);
		return $plugins[$plugin_id];
	}

	return false;
}

I'm assigning this to 1.8.7 so that we look at it and re-evaluate due to changing how we load plugins.

[srokap]

It seems I've messed up some of old pull requests after rebasing - it should be single commit there. Changes were only to this function. It's obviously better to have single mechanism than separate.

[cash]

Generally, we are going to load a plugin from id when it is already an active plugin. We have already loaded active plugins. If we stored the plugin objects in a cache, we could hit the cache before trying to hit the database. It's likely it would never go to database. This would be better than having a static cache within this single function.

[srokap]

Exactly, when having plugins in entity cache only thing left to do is to store and use mapping between plugin_id and GUID somehow. This ticket becomes obsolete if that's taken care of and such solution is much better than optimizing single function.

See: #4313 and #4543