Skip to content

Faster archiving by calculating the recursive count only if needed #7468

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

Merged
merged 3 commits into from
Mar 18, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Mar 18, 2015

The method ArchiveProcessor::aggregateDataTableRecords counts by default the number of rows recursive. I noticed this takes a lot of time especially on large dataTables: https://github.com/piwik/piwik/blob/2.12.0-b7/core/ArchiveProcessor.php#L220

We're calling this method 17 times but it is only actually used once. Therefore I added a parameter to disable it (not enabled by default to stay BC. I failed to find a better solution. Only solution I could think of alternatively would be to add another method that does not count recursive.

In the following example run you can see that it takes 13.5 seconds (with xhprof and only no_builtins flag) which is 15% of the total time to get the rows count recursive. And this is only for 5 records. Those 17 calls easily make 30 records. Meaning we will save a lot of time when generating week, month, year and range archives.

getrowscountrecursive
)

@tsteur tsteur added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Mar 18, 2015
@@ -475,7 +470,7 @@ private function isSummableColumn($columnName)
*/
public function sumRow(Row $rowToSum, $enableCopyMetadata = true, $aggregationOperations = false)
{
foreach ($rowToSum->getColumnsRaw() as $columnToSumName => $columnToSumValue) {
foreach ($rowToSum->getColumns() as $columnToSumName => $columnToSumValue) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was just something that I noticed while working on it. getColumnsRaw and getColumns does now do the same since we removed callable columns a few weeks ago. I can remove it from the PR if it annoys anyone

@tsteur tsteur added c: Performance For when we could improve the performance / speed of Matomo. Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Mar 18, 2015
mattab pushed a commit that referenced this pull request Mar 18, 2015
Faster archiving by calculating the recursive count only if needed
@mattab mattab merged commit 8dc8976 into master Mar 18, 2015
@mattab
Copy link
Member

mattab commented Mar 18, 2015

👍 refs #4768

@mattab mattab added this to the Piwik 2.12.0 milestone Mar 18, 2015
@tsteur tsteur deleted the faster_archiving branch March 18, 2015 05:42
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
Development

Successfully merging this pull request may close these issues.

2 participants