Skip to content
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

All websites dashboard - improve performance when thousands of websites #7693

Merged
merged 9 commits into from Apr 22, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Apr 17, 2015

fixes #6809
This improves the performance of the All Websites Dashboard. It includes improvements on several layers:

  • Instead of fetching all sites into JS we do now only fetch the sites that we need to display. This means we will do a new request whenever someone sorts, searches, ...
  • Before we always requested the MultiSites.getAll twice whenever the websites dashboard was loaded. One to get the current dashboard and one request to get the previous one (this was needed for a hover tooltip, the past_data.total.nb_visits). We do now only one request as the MultiSites.getAll already always requested the past data anyway. I do now expose this total visits value as well
  • I made MultiSites.getAll on the server much faster. Eg before it took easily > 300 seconds to perform one MultiSites.getAll request when having 33k sites. This is now done in less than 10 seconds. If only a few sites are archived, and browser archiving is disabled, the performance improved from > 60 seconds to < 5 seconds. Basically a problem was that we first created eg 100K DataTables, 1mio Rows, ... just to merge them afterwards together via mergeChildren to 1 DataTable and 33k Rows. This took a long time. Instead we want to directly build the merged dataTable. There are many changes in this pull request and each of them has actually a huge impact on MultiSites.getAll performance.

@tsteur tsteur added the Needs Review PRs that need a code review label Apr 17, 2015
$result[$key] = $this->createOrderedIndex($metadataNamesToIndexBy);
if (empty($metadataNamesToIndexBy)) {
foreach ($indexKeyValues as $key) {
$result[$key] = array();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to myself, use array_fill_keys or so

@tsteur tsteur changed the title All websites dashboard - improve performance when thousands of websites #6809 All websites dashboard - improve performance when thousands of websites Apr 17, 2015
@mattab mattab added this to the Piwik 2.13.0 milestone Apr 17, 2015
@tsteur tsteur force-pushed the 6809_2 branch 2 times, most recently from ad55d37 to acb5271 Compare April 21, 2015 02:01
@@ -157,21 +161,65 @@ public function useSubtable($idSubtable)
*/
public function make($index, $resultIndices)
{
$keyMetadata = $this->getDefaultMetadata();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of time was spent in setting and transforming metadata. This is much faster now. Eg before we did set a table metadata like array('period' => '2012-12...', 'site' => 5) and they were afterwards "transformed" to array('period' => Period, 'site' => new Site()).

$keyMetadata[$resultIndex] = new Site($label);
} elseif ($resultIndex === DataTableFactory::TABLE_METADATA_PERIOD_INDEX) {
$keyMetadata[$resultIndex] = $this->periods[$label];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small code clarity item: I think this might be clearer in a private method, ie, $keyMetadata[$resultIndex] = $this->createTableIndexMetadata($resultIndex, $label);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@mattab
Copy link
Member

mattab commented Apr 22, 2015

Needs a rebase & then +1 to merge it before the beta/RC

mattab pushed a commit that referenced this pull request Apr 22, 2015
All websites dashboard - improve performance when thousands of websites
@mattab mattab merged commit 6921330 into master Apr 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants