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

Made reports faster when flat=1 is used. #7409

Merged
merged 2 commits into from Mar 12, 2015
Merged

Made reports faster when flat=1 is used. #7409

merged 2 commits into from Mar 12, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Mar 11, 2015

Also replaceColumnNames is now queued again which should bring a performance boost in general for not flattened and not expanded reports.

Eg in my dev vm A huge first level getPageUrls report loads in < 300ms. Half of that time is spent in Twig, about 1/4 in bootstrapping etc. We do now spend more time in JS (300-400ms on a fast computer with a fast browser) after the report is fetched than it takes to fetch it.

tsteur and others added 2 commits March 11, 2015 01:34
Also replaceColumnNames is now queued again which should bring
a performance boost in general.
@@ -337,6 +323,36 @@ public function getSocials($idSite, $period, $date, $segment = false, $expanded
$this->setSocialIdSubtables($dataTable);
$this->removeSubtableMetadata($dataTable);

if ($flat) {
$urlsTable = Archive::createDataTableFromArchive(Archiver::WEBSITES_RECORD_NAME, $idSite, $period, $date, $segment, $expanded, $flat);
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we need a special handling to make it fast as getSocials does not really have a subtable...

Copy link
Member

Choose a reason for hiding this comment

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

can you refactor this special handling in own method?
(or maybe even own filter if it makes sense)

@tsteur tsteur added the Needs Review PRs that need a code review label Mar 11, 2015
@tsteur tsteur added this to the Piwik 2.12.0 milestone Mar 11, 2015
if (Common::getRequestVar('flat', 0, 'int') === 1) {
$view->requestConfig->filter_excludelowpop = 'nb_hits';
} else {
$view->requestConfig->filter_excludelowpop = Metrics::INDEX_PAGE_NB_HITS;
Copy link
Member

Choose a reason for hiding this comment

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

un-related to this PR, but +1 toconsider removing this mix of Metrics ID and Named metrics... could you create an issue for this? (to store named metrics in archives rather than stored metrics ID)

@mattab
Copy link
Member

mattab commented Mar 12, 2015

Feedback

  • Did you check that all APIs have a System test case that tests with flat=1 and expanded=1? I'm almost sure that all APIs are tested as system test without flat=1 but because we touch here flat=1 and expanded=1 it would be good to confirm that these are covered in system tests
  • Nice performance boost!!

mattab pushed a commit that referenced this pull request Mar 12, 2015
Made for reports faster when flat=1 is used.
@mattab mattab merged commit 1b8320f into master Mar 12, 2015
@mattab mattab changed the title Made for reports faster when flat=1 is used. Made reports faster when flat=1 is used. Mar 22, 2015
@mattab mattab added the c: Performance For when we could improve the performance / speed of Matomo. label Mar 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Performance For when we could improve the performance / speed of Matomo. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants