Performance issues in sites with a lot of users groups#11853
Merged
rdeutz merged 16 commits intojoomla:stagingfrom Aug 31, 2016
Merged
Performance issues in sites with a lot of users groups#11853rdeutz merged 16 commits intojoomla:stagingfrom
rdeutz merged 16 commits intojoomla:stagingfrom
Conversation
| * | ||
| * @return array | ||
| * | ||
| * @since DEPLOY_VERSION |
Contributor
There was a problem hiding this comment.
@phproberto it is __DEPLOY_VERSION__ ;) I have send you a PR to fix that phproberto#4. Thanks.
Contributor
|
Result.
note: I have not exiplite tested the performance. |
Contributor
|
I have tested this item ✅ successfully on a4d42a4 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853. |
__DEPLOY_VERSION__ for the new code
Contributor
Author
|
Merged. Thanks @zero-24 :) |
Contributor
|
I have tested this item ✅ successfully on 3f1c170 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853. |
Member
|
I have tested this item ✅ successfully on This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853. |
Contributor
|
I have broken travis :( If that is fixed we can set it RTC. phproberto#5 Thanks 👍 |
Sorry i have implemented to much here.
Contributor
Author
|
👍 Merged @zero-24 |
Contributor
|
I have tested this item ✅ successfully on 2d15209 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853. |
Contributor
|
Since the last change was only a CS I will move this to RTC Thank you @phproberto |
Contributor
|
I have tested this item ✅ successfully on 2d15209 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11853. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
#__usergroupstable should really extendJTableNestedbut it's there untouched for years. The main issue is thatlevelof groups is not stored in database when a group is saved and that forces that any operation that requires to retrieve a list of groups with their levels uses a query like:That uses MySQL to calculate the groups level but it's a really expensive query (~0.5 seconds for 1500 groups). Most Joomla sites have < 10 user groups so the performance issues are not noticeable but as you will see in the benchmarks below this speeds things a lot.
In the near future we should modify database table to add columns, etc. but I don't have the time now to deal with such a big change and the issues it may cause.
This PR replaces the method to retrieve groups levels & paths. It really improves data reliability because it uses the group tree to populate that data which is correct even when something went wrong and lft & rgt values are wrong.
Summary of Changes
The new method to populate groups levels & paths is quite simple: the list of groups is retrieved with a simple query (~0.011 seconds for 1500 groups) like:
then a recursive php function traverses the group tree until it reaches
parent_id= 0 which is assigned to level 0. Additionally when it reaches that level it starts to populate the group path. Data is sent back to the recursive function so all the children groups get the correct path & level.This PR gives us other benefits:
JHelperUsergroups::getInstance()) that can be used to deal with user groups. Using a singleton ensures that no matter how many times you need to get the available user groups in the same page load (it happens a lot in com_users backend managements) it will only retrieve the list once.JHelperUsergroupsbecomes the central place to deal with existing groups.Benchmarks
I did a benchmark for each function I have modified. Each benchmark value shown here is an average of 50 measurements done on a Joomla site with 1500 user groups. All the queries benchmarked use
SQL_NO_CACHEto ensure that no cache affects data.Benchmark system information:
Performance improvements for 1500 user groups:
Testing Instructions
Perform any additional test you think may be applicable.
If you want to test performance and you need to create groups I have created a small shit snippet:
You can place that in backend isis template index.php and it will allow you to create groups with urls like:
Remember to remove that shit when you are finished testing this and don't be stupid to use it in other places because is not secure :P
Documentation Changes Required
Example usage of
JHelperUsergroupas singleton:Example usage of
JHelperUsergroupas standard instance: