Faster access loading - Make dashboard etc faster when having thousands of sites #7641
Conversation
This file contains 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
…is superuser and has many sites
|
|
||
| // if the Auth wasn't set, we may be in the special case of setSuperUser(), otherwise we fail | ||
| if (is_null($this->auth)) { |
There was a problem hiding this comment.
This if is not need as we would check for hasSuperUserAccess afterwards anyway
tsteur
added
the
c: Performance
| if ($bool) { | ||
| $this->reloadAccessSuperUser(); | ||
| $this->makeSureLoginNameIsSet(); |
There was a problem hiding this comment.
Should we resetSites in this case too?
There was a problem hiding this comment.
Don't think it's needed. It would rather end up in loading more sites eg if superuser sites were already set we do not need to reload them.
There was a problem hiding this comment.
You're right, when super user access is set, only the 'superuser' element in idsitesByAccess or whatever is used.
|
In general, looks good to me, can merge when comment threads are dealt w/. |
added 5 commits
April 16, 2015 19:17
…heduled task to keep it out of getUserPreference method.
diosmosis
added a commit
that referenced
this pull request
Apr 17, 2015
Performance optimization: Load access levels in Access singleton lazily, making dashboard and other operations that do not require access faster when having thousands+ sites.
|
fyi: the build was broken at this commit: https://travis-ci.org/piwik/piwik/jobs/58851183 edit: any idea why it broke the build? |
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.
Currently, we do always load the sites a user has access to even though when never requested. Eg most dashboard widgets do not need to fetch all siteIds from the database when someone is a superuser. When someone is a user we still often need to do it but often users maybe do not have access to eg all 50k sites so it should be fast.
Basically we want to lazy load the sites a user has access to. This improves performance quite a bit especially when opening the dashboard and when many requests to request 50k sites in parallel. On a fast server the difference is not that obvious but one can feel it is faster.