Performace: Order of checks in local_template_import_before_standard_top_of_body_html

[mudrd8mz]

The function local_template_import_before_standard_top_of_body_html() is executed on every single page load. It is essential that is performs effectively without adding unnecessary load to the server.

For that reason, I would suggest to re-order the early-exit checks so that first are evaluated those that do not need any further processing. For example the if (!strpos($PAGE->url, '/course/view.php')) should be evaluated as a very first one probably, followed by the $COURSE->id check and only then checks involving the capabilities.

Additionally, you may consider to display the popup only if the editing mode is enabled, effectively checking for $PAGE->user_is_editing().

[stemiwe]

so you would recommend placing these checks even above checking whether the plugin is activated or not? (if (get_config('local_template_import', 'activate') !== "1"))

as for the second suggestion, i think it is better to display the popup no matter if editing mode is on or not, as this would just require one additional click and might confuse some users.

[mudrd8mz]

Well. It depends. From the functionality perspective, it does not really matter which of these checks is executed in which order right? All of these checks are equal in the sense that any of them may make the function to exit / return early without any processing. So my point is that first should be checked those criteria that have no impact on the site performance.

Now, calls to get_config() should use the config cache so it does not necessarily lead to a DB query. But still is is more expensive than e.g. checking the PAGE url (which is plain string comparison without any I/O involved).

And because this function is called on every single request by any visitor of the site, even small micro-optimisations may have significant impact.

[stemiwe]

sounds good, thanks for clarifying :)