diff options
author | Thomas Steur <thomas.steur@gmail.com> | 2015-02-09 01:54:19 +0300 |
---|---|---|
committer | Thomas Steur <thomas.steur@gmail.com> | 2015-02-10 03:44:58 +0300 |
commit | 6c0db645eafb257eff599d8c85f99537df34d379 (patch) | |
tree | 0e2264cafc6b5187216fba1900ebb44904e0f7d3 /plugins | |
parent | 36d87593bc0dfbad4126ef1ed4845be3f30e54dc (diff) |
refs #6809 a couple of performance improvements for the all websites report.
I tested it with 4k websites. The change made it only a couple of seconds faster
but requesting 4k websites would still take 4-8 seconds even if only 10 sites of
them are actually requested. For example before we did request each site 3 times
(12k times in total and triggered 12k events etc) which should be no longer the case.
The last `clearSiteCache` was there to free some memory but I think it shouldn't
be problematic to remove it. Also I removed one 'sort' as a lot of time is wasted
there. It should be in theory sorted afterwards anyway again. Need to see test
results whether this change is good or not.
Diffstat (limited to 'plugins')
-rw-r--r-- | plugins/API/ProcessedReport.php | 5 | ||||
-rwxr-xr-x | plugins/MultiSites/API.php | 109 |
2 files changed, 58 insertions, 56 deletions
diff --git a/plugins/API/ProcessedReport.php b/plugins/API/ProcessedReport.php index bd1a885f4d..7c4bd46c9e 100644 --- a/plugins/API/ProcessedReport.php +++ b/plugins/API/ProcessedReport.php @@ -674,8 +674,9 @@ class ProcessedReport // if we handle MultiSites.getAll we do not always have the same idSite but different ones for // each site, see https://github.com/piwik/piwik/issues/5006 $idSiteForRow = $idSite; - if ($row->getMetadata('idsite') && is_numeric($row->getMetadata('idsite'))) { - $idSiteForRow = (int) $row->getMetadata('idsite'); + $idSiteMetadata = $row->getMetadata('idsite'); + if ($idSiteMetadata && is_numeric($idSiteMetadata)) { + $idSiteForRow = (int) $idSiteMetadata; } $prettyValue = self::getPrettyValue($formatter, $idSiteForRow, $columnName, $columnValue, $htmlAllowed = false); diff --git a/plugins/MultiSites/API.php b/plugins/MultiSites/API.php index fbc3068d2b..144ef15d7c 100755 --- a/plugins/MultiSites/API.php +++ b/plugins/MultiSites/API.php @@ -18,7 +18,6 @@ use Piwik\Period\Range; use Piwik\Piwik; use Piwik\Plugins\Goals\Archiver; use Piwik\Plugins\SitesManager\API as APISitesManager; -use Piwik\Plugins\SitesManager\Model as ModelSitesManager; use Piwik\Scheduler\Scheduler; use Piwik\Site; @@ -87,13 +86,14 @@ class API extends \Piwik\Plugin\API { Piwik::checkUserHasSomeViewAccess(); - $idSites = $this->getSitesIdFromPattern($pattern); + $sites = $this->getSitesIdFromPattern($pattern, $_restrictSitesToLogin); - if (empty($idSites)) { + if (empty($sites)) { return new DataTable(); } + return $this->buildDataTable( - $idSites, + $sites, $period, $date, $segment, @@ -106,27 +106,41 @@ class API extends \Piwik\Plugin\API /** * Fetches the list of sites which names match the string pattern * - * @param $pattern + * @param string $pattern + * @param bool $_restrictSitesToLogin * @return array|string */ - private function getSitesIdFromPattern($pattern) + private function getSitesIdFromPattern($pattern, $_restrictSitesToLogin) { - $idSites = 'all'; if (empty($pattern)) { - return $idSites; - } - $idSites = array(); - $sites = Request::processRequest('SitesManager.getPatternMatchSites', - array('pattern' => $pattern, - // added because caller could overwrite these - 'serialize' => 0, - 'format' => 'original')); - if (!empty($sites)) { - foreach ($sites as $site) { - $idSites[] = $site['idsite']; + + // First clear cache + Site::clearCache(); + + /** @var Scheduler $scheduler */ + $scheduler = StaticContainer::getContainer()->get('Piwik\Scheduler\Scheduler'); + // Then, warm the cache with only the data we should have access to + if (Piwik::hasUserSuperUserAccess() + // Hack: when this API function is called as a Scheduled Task, Super User status is enforced. + // This means this function would return ALL websites in all cases. + // Instead, we make sure that only the right set of data is returned + && !$scheduler->isRunningTask() + ) { + APISitesManager::getInstance()->getAllSites(); + } else { + APISitesManager::getInstance()->getSitesWithAtLeastViewAccess($limit = false, $_restrictSitesToLogin); } + // Both calls above have called Site::setSitesFromArray. We now get these sites: + $sitesToProblablyAdd = Site::getSites(); + } else { + $sitesToProblablyAdd = Request::processRequest('SitesManager.getPatternMatchSites', + array('pattern' => $pattern, + // added because caller could overwrite these + 'serialize' => 0, + 'format' => 'original')); } - return $idSites; + + return $sitesToProblablyAdd; } /** @@ -145,8 +159,11 @@ class API extends \Piwik\Plugin\API public function getOne($idSite, $period, $date, $segment = false, $_restrictSitesToLogin = false, $enhanced = false) { Piwik::checkUserHasViewAccess($idSite); + + $sites = $this->getIdSite($idSite); + return $this->buildDataTable( - $idSite, + $sites, $period, $date, $segment, @@ -156,37 +173,27 @@ class API extends \Piwik\Plugin\API ); } - private function buildDataTable($idSitesOrIdSite, $period, $date, $segment, $_restrictSitesToLogin, $enhanced, $multipleWebsitesRequested) + private function getIdSite($idSite) { - $allWebsitesRequested = ($idSitesOrIdSite == 'all'); - if ($allWebsitesRequested) { - // First clear cache - Site::clearCache(); - /** @var Scheduler $scheduler */ - $scheduler = StaticContainer::getContainer()->get('Piwik\Scheduler\Scheduler'); - // Then, warm the cache with only the data we should have access to - if (Piwik::hasUserSuperUserAccess() - // Hack: when this API function is called as a Scheduled Task, Super User status is enforced. - // This means this function would return ALL websites in all cases. - // Instead, we make sure that only the right set of data is returned - && !$scheduler->isRunningTask() - ) { - APISitesManager::getInstance()->getAllSites(); - } else { - APISitesManager::getInstance()->getSitesWithAtLeastViewAccess($limit = false, $_restrictSitesToLogin); + $idSite = (int) $idSite; + $sites = array(APISitesManager::getInstance()->getSiteFromId($idSite)); + + return $sites; + } + + private function buildDataTable($sitesToProblablyAdd, $period, $date, $segment, $_restrictSitesToLogin, $enhanced, $multipleWebsitesRequested) + { + $idSites = array(); + if (!empty($sitesToProblablyAdd)) { + Site::setSitesFromArray($sitesToProblablyAdd); + foreach ($sitesToProblablyAdd as $site) { + $idSites[] = $site['idsite']; } - // Both calls above have called Site::setSitesFromArray. We now get these sites: - $sitesToProblablyAdd = Site::getSites(); - } else if (is_array($idSitesOrIdSite)) { - $model = new ModelSitesManager(); - $sitesToProblablyAdd = $model->getSitesFromIds($idSitesOrIdSite); - } else { - $sitesToProblablyAdd = array(APISitesManager::getInstance()->getSiteFromId($idSitesOrIdSite)); } // build the archive type used to query archive data $archive = Archive::build( - $idSitesOrIdSite, + $idSites, $period, $date, $segment, @@ -210,8 +217,7 @@ class API extends \Piwik\Plugin\API // get the data // $dataTable instanceOf Set $dataTable = $archive->getDataTableFromNumeric($fieldsToGet); - - $dataTable = $this->mergeDataTableMapAndPopulateLabel($idSitesOrIdSite, $multipleWebsitesRequested, $dataTable); + $dataTable = $this->mergeDataTableMapAndPopulateLabel($idSites, $multipleWebsitesRequested, $dataTable); if ($dataTable instanceof DataTable\Map) { foreach ($dataTable->getDataTables() as $table) { @@ -237,11 +243,11 @@ class API extends \Piwik\Plugin\API $dataTable->setMetadata(self::getLastPeriodMetadataName('date'), $lastPeriod); } - $pastArchive = Archive::build($idSitesOrIdSite, $period, $strLastDate, $segment, $_restrictSitesToLogin); + $pastArchive = Archive::build($idSites, $period, $strLastDate, $segment, $_restrictSitesToLogin); $pastData = $pastArchive->getDataTableFromNumeric($fieldsToGet); - $pastData = $this->mergeDataTableMapAndPopulateLabel($idSitesOrIdSite, $multipleWebsitesRequested, $pastData); + $pastData = $this->mergeDataTableMapAndPopulateLabel($idSites, $multipleWebsitesRequested, $pastData); // use past data to calculate evolution percentages $this->calculateEvolutionPercentages($dataTable, $pastData, $apiMetrics); @@ -259,14 +265,9 @@ class API extends \Piwik\Plugin\API $dataTable->queueFilter('ColumnDelete', array('label')); } - Site::clearCache(); - // replace record names with user friendly metric names $dataTable->queueFilter('ReplaceColumnNames', array($columnNameRewrites)); - // Ensures data set sorted, for Metadata output - $dataTable->filter('Sort', array(self::NB_VISITS_METRIC, 'desc', $naturalSort = false)); - // filter rows without visits // note: if only one website is queried and there are no visits, we can not remove the row otherwise // ResponseBuilder throws 'Call to a member function getColumns() on a non-object' |