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

Support for secondary column support and faster datatable sort #8449

Merged
merged 4 commits into from Aug 18, 2015
Merged

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jul 28, 2015

fixes #7401

Made sort faster by using builtin php methods. Currently, we sort via usort in PHP. This is quite slow especially when we sort many rows, eg 25k rows or more. The time needed to sort depends a lot on the data, the column to sort, etc. that's why it is hard to say how much performance improvement we will gain. On 25k rows it can drop from eg before about 900ms to 500ms.

As a side effect the code is much better tested and it does now use a secondary column on either nb_visits or label if possible see #7401

The sort flags SORT_NATURAL and SORT_FLAG_CASE were added in PHP 5.4 which this pull request requires, therefore this goes into Piwik 3.0

Original PR was made in #7420

refs #4768

@tsteur tsteur added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Performance For when we could improve the performance / speed of Matomo. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jul 28, 2015
@tsteur tsteur added this to the 3.0.0 milestone Jul 28, 2015
@tsteur
Copy link
Member Author

tsteur commented Jul 29, 2015

FYI: Need to rerun test once the UI test run on PHP 5.4 in this branch

@tsteur
Copy link
Member Author

tsteur commented Jul 29, 2015

One can see how the secondary column sort is quite useful. Eg here when rows are having same values it sorts by pageviews: http://builds-artifacts.piwik.org/ui-tests.fast_sort2/14483.7/ActionsDataTable_column_sorted and http://builds-artifacts.piwik.org/ui-tests.3.0/14505.7/GoalsTable_goals_table_ecommerce

Link to UI tests: http://builds-artifacts.piwik.org/ui-tests.3.0/14521.7/

@tsteur tsteur added 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 Jul 29, 2015
@tsteur
Copy link
Member Author

tsteur commented Jul 30, 2015

The tests in SortTest that were in original PR were already merged previously. There are tests for Sorter class now to make sure it works as expected. The sort filter itself does not really contain any logic so did not add more tests as there are already

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. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant