-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
various performance tweaks #14624
various performance tweaks #14624
Conversation
@@ -178,10 +178,6 @@ protected function getValue($name) | |||
$value = Db::fetchOne('SELECT option_value FROM `' . Common::prefixTable('option') . '` ' . | |||
'WHERE option_name = ?', $name); | |||
|
|||
if ($value === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I didn't get why there would be per request eg 140 calls to Option::get()
from like 15 different places and 79 calls where actually hidding the DB. Until I realised for many of these values there is no value configured on option
table and it would fetch the value from the DB again and again. Some option names it was fetching over 30 times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a value is set later, it should go through ::set() anyway and be invalidated.
core/Plugin.php
Outdated
@@ -438,6 +438,10 @@ public function getMissingDependencies($piwikVersion = null) | |||
return array(); | |||
} | |||
|
|||
if (!SettingsPiwik::isSystemValidationEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small tweak to not having to execute this on very request as we know on production there are no missing dependencies. Happy to change the name of the setting. We might want to add something like this eventually to more places as we know our system is correctly configured. Saves eg 1% per tracking request.
core/Plugin/Manager.php
Outdated
@@ -319,12 +319,14 @@ public function isPluginLoaded($name) | |||
*/ | |||
public function readPluginsDirectory() | |||
{ | |||
$isSystemValidationEnabled = SettingsPiwik::isSystemValidationEnabled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here as in https://github.com/matomo-org/matomo/pull/14624/files#r300292474
We know the plugin structures are valid and therefore we don't want to execute this on every request as it's a lot of stat calls.
core/SettingsPiwik.php
Outdated
*/ | ||
public static function isSystemValidationEnabled() | ||
{ | ||
return (bool) Config::getInstance()->General['enable_system_validation']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as mentioned happy to change name of the setting
@@ -266,8 +266,7 @@ public function render() | |||
|
|||
$this->loginModule = Piwik::getLoginPluginName(); | |||
|
|||
$user = APIUsersManager::getInstance()->getUser($this->userLogin); | |||
$this->userAlias = $user['alias']; | |||
$this->userAlias = $this->userLogin; // can be removed in Matomo 4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not using the alias anymore, and we will even remove it in Matomo 4. Couldn't find any usage in our plugins of this...
Before we were requesting the user
DB on every VIEW render. Especially when we render the visitor log we might have hundreds or thousands of VIEW renders saving hundreds or thousands of calls to the user
DB
core/Plugin/Manager.php
Outdated
@@ -319,6 +319,10 @@ public function isPluginLoaded($name) | |||
*/ | |||
public function readPluginsDirectory() | |||
{ | |||
if (!empty($GLOBALS['MATOMO_PLUGIN_NAMES_AVAILABLE'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this along with the other related changes saves 1-2% of each tracking request. Eventually we could add more "caching" around this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In $matomoDir/bootstrap.php
someone could basically configure which plugin names are available in an array.
No description provided.