[#14279] - Improve performance with assets
- Closed
- 3 Mar 2017
- Medium
- Build: staging
- # 14279
- Diff
- csthomas:improve_assets
User tests: Successful: Unsuccessful:
Summary of Changes
-
Do not add default values from com_content component asset to new assets, better use inheritance
{}. -
Leaf asset (last-child, ex: com_content.article.1) can be replaced by parent asset if leaf asset does not change any permission.
If article, module or ucm_content table has reference to leaf asset which has rules as {} (means inherits all rules from parent asset) then this table may reference to parent asset instead.
Example for com_content:
- 1000 articles in 10 categories have 1011 assets = 1000 (com_content.article.xxxx) + 10 (com_content.category.xx) + 1 (com_content)
After current PR com_content assets may be reduced to:
- 1000 articles in 10 categories will have 11 assets = 10 (com_content.category.xx) + 1 (com_content)
Notice: If user won't set article permissions as inherit for all then there won't be any benefits. (old behaviour)
It seems that lots of users do not use too often article permission tab or change its value so
this PR would improve performance a lot.
Current problem:
- before this PR joomla does not use (on creating article) inherit value by default.
That means that exist articles won't be automatically improved.
But every new article with all permission inherited from parent won't create a new row for#__assetstable.
How to fix old articles?
Re-save each one or use a few sql queries to fix that.
[I may add it later]
Testing Instructions
- Install joomla with some contents, like sample - testing data
- Change article permissions few times, set all to inherit. (always save article after changes)
- On phpmyadmin you can see that on changed article (with set all as inherit), means rules =
{}
article refers to category asset and recreate own asset if you change at least one permission to notinherit. - Set permission rules to categories, articles and check whether values are preserved after saved.
- Check if permission is correct on frontend for modules and articles.
Documentation Changes Required
Maybe add some notes about reference to parent assets when article/module... use full inherited permissions.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_config Libraries |
| Labels |
Added:
?
|
||
I have changed it and a few other lines.
Now it is more safe.
Foreign assets work only for tables content and module but could be expanded any time.
| Category | Administration com_config Libraries | ⇒ | Administration com_config com_fields Libraries |
| Title |
|
||||||
| Category | Administration com_config Libraries com_fields | ⇒ | Administration com_config Libraries |
| Category | Administration com_config Libraries | ⇒ | Administration com_config Front End com_content Libraries |
| Category | Administration com_config Libraries Front End com_content | ⇒ | Administration com_config Front End com_content Libraries Unit Tests |
| Labels |
Added:
?
|
||
Are you removing actual asset or just rules saving (my comment above is just about rules) ? With actual asset removed it is likely performance will decrease on the reading side.
As a general rule, an optimized mysql query will always be faster than php if they are both doing the same tasks. So unless you have millions of assets to choak mysql, removing asset for each item article will most likely lead to perfomance degradation on the reading side (mysql can handle mentioned 10.000 rows just fine). Reason is that without asset id there is no lft-rgt to start self joins to get parent levels in single query and since we need to do multiple queries as there still might be item level permission set, then if nothing is found do another query to check parents.
But to put speculations aside, we need benchmark test this approach (vs. previous one) on a large database to get more definite answers.
Reason is that without asset id there is no lft-rgt to start self joins to get parent levels ...
You do not understand that PR correctly. I do not remove assets. I replace for example 10,000 article assets by 100.
The preload() methods existed before and I do not touch it, there is a lots of method in access.php which was added and was before J3.7.
@klas Your PR change assets in other place. Our changes can coexists. I see 3 layers in that stuff:
- How looks the structure of asset. (@klas your PR)
- How we request for asset, "preloading" (methods from
access.php). - Which assets we need to work (my PR).
My PR do not remove assets, but use for ex. category asset if do not need to create other one.
On Joomla 3.7 each article create own row in assets table like "com_content.artcile.12345".
Usual it may have the same rules as in "com_content.category.1" (if article catid=1).
My PR only do not create row "com_content.article.12345" but link to existed row "com_content.category.1".
If you re-save old article, or change permissions then it will recheck possibility to use "com_content.category.1" or uses/creates asset "com_content.article.12345" otherwise.
This allow to eliminate duplication which is not needed.
I have seen there is some php trick with md5(rules) in order to not duplicate rows in php memory.
This PR doing such thing in database.
About front end speed.
Joomla with missing assets 'com_content.article.12345' will be slower because it can not find own asset and therefore preloading won't work.
But my PR has asset but usual with different name like 'com_content.category.1' instead 'com_content.article.12345'.
I created a function for B/C to translate 'com_content.article.12345' to real value of asset_id.
But we do not need to use asset_name in com_content article model. We have column asset_id from #__content table.
Component com_content and com_modules stop using asset_name like "com_content.article.12345" and base on own column asset_id.
Reason is that without asset id there is no lft-rgt to start self joins to get parent levels ...
Once more.
In Joomla 3.7 article is linked to asset_name com_content.article.12345 and then join to the parent asset with name com_content.category.1 and next parent levels ...
In my PR article can be linked with asset_id which has asset_name com_content.category.1 and then join to parents as before.
As you can see we can skip first asset and start from second (com_content.category.1).
Ok, thnx for explanation. This is crucial:
My PR only do not create row "com_content.article.12345" but link to existed row "com_content.category.1".
it is fine then as existing #__content asset_id -> #__assets id relation still exists, I thought you are removing it.
Regarding asset names - they should be dropped totally (not just here), in my opinion, I did exactly the same in my pr.
Still some benchmarks and functional test would be welcome, would be good if we can create a set of such tests together so we can compare without guessing.
as this PR is about large sites will help to get sampledata for large sites to test.
Unfortunately this change is not B/C, since it changes the relations in the DB. If an existing extension deletes a module or an article, it will expect the asset to belong to the article or module and delete it also. In that case the permissions system would be pretty much broken.
General this is not a problem for now.
Take a look at:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/table/table.php#L988
It is a change of the DB relations from a 1:1 relation to a n:1 relation. That is not B/C and it doesn't matter if our JTable wrapper class would properly handle this. Otherwise we could have renamed attribs in '#__content' to params already. It breaks all scripts out there, that write directly to the table.
This PR for J3.7.x is closed.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-03-03 14:24:45 |
| Closed_By | ⇒ | csthomas |
This makes perfect sense.
Regarding code side I wonder if changes in rules class are really needed? https://github.com/joomla/joomla-cms/pull/14279/files#diff-4ddb9f5ef277bd4fbb023f65c9a9c693
Existing classes should be able to handle reading permissions when they are unset on the item level. This change could have siddeffects in other operations as this method is used often in various contexts.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14279.