User tests: Successful: Unsuccessful:
[UPDATED (5.1.2017) - see second paragraph]
Those are some performance optimizations for the libraries/cms folder. If you like those, there are plenty more where those came from.
Since I don't know if you guys are open to these kinds of PR's. I am awaiting the developments of my open PR's before I do more.
[Update 5.1.2017]
In order to lighten this PR up, I have introduced 6 sub-PRs, which are easier to digest and mostly with very specific changes. In order to continue with this one, those ones should be tested/reviewed/merged first. After that, I can resolve the conflicts and there should finally be only a few changes left.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
?
|
Yes, I noticed... :) Will fix errors.
on it...
That about sums this PR up. No new commits, as far as optimizations are concerned.
Let me know if you need me to change anything.
Would you mind motivating the other types of performance improvements as well?
@sovainfo I'll gladly point out my motivation for this.
Generally, all of the improvements are based on modern PHP best practices.
Most of them are dropping function calls in favor of using cheaper (in terms of cpu usage and memory) implementations.
Often, readability is also improved as a welcome side-effect of this.
Other things like the optimized if-conditions, are moving the cheaper conditions to the beginning of the conditions, in order to not invoke more expensive operations, if not necessary.
Of course there are some situations, where optimization is not possible, because of the necessity of maintaining PHP 5.3 compatibility. (Are there really people still running 5.3???)
IMHO, Joomla 4, whenever it comes out, should radically drop the complete 5.* line of PHP and have a minimum of PHP 7.0. Because 5.6 is going into 'security fixes only' mode on 1.1.2017 - see here
That should about sum it up.
As you have probably noticed, this PR is only focusing on the files in libraries/cms + it's subfolders, as I though it was a good starting point. If you guys are OK with those changes, I'll gladly expand the implementation of that kind of optimizations and cleanup to the whole project.
Thanks, was wondering about the optimization of if-conditions.
The costs of the condition shouldn't be the only factor. Sounds like you want to ignore the likely hood of the conditions, together they should determine the order.
This assumes it was given thought in the first place.
Surprisingly almost 14% of submitted user statistics were for PHP 5.3
I personally question how many of those were submitted by developers doing testing
https://developer.joomla.org/about/stats.html
@photodude yes, that's a valid question. I too think the developer tests are tinkering with real statistics. I wonder if there should be some kind of 'isDeveloper' switch, which developers could set, in order to not have unwanted side effects like this.
Personally I think almost no dev tests with 5.3. Devs are the ones with the PHP7 servers
But users really are still using PHP 5.3. If you ask why, it's because they don't care about underlying systems and just trust their hosters. And those hosters don't want to risk breaking sites by upgrading the PHP version.
Part of it is we didn't want to make a distinction between dev and production environments in the statistics. It honestly doesn't matter to us either (well, that's my opinion anyway, YMMV). That's in part why you get the nag message versus default enabled (every fresh install I do I immediately hit never on it). We have provisions in the stats collection making it possible to report things that nobody will be using in production (i.e. our 4.0 branch doesn't get rejected, if you're compiling against PHP master which has a 7.2 version number right now it's accepted, etc.) so that too can skew things because it favors either ambitious or curious folks who want to try things out on bleeding edge technology and that can also be used as an indicator to see what folks are doing.
@Bakual I don't know about the devs testing with 5.3, but if the number really depicts users still using 5.3, then that's one more reason to require PHP 7.0 for Joomla 4, because if Joomla 4 would be released in 2017, then PHP 5.6 would already be in security only-fixes mode for about a year until its final EOL end of 2018.
This is Joomla's chance to help hosters get rid of old versions and also be able to use the newer technologies and speed provided by the PHP 7 series.
I don't think it's reasonable to raise the minimum PHP requirement for Joomla 4 to PHP 7, but PHP 5,.6 for sure. If it is released next year that is.
By the same logic you should raise the minimum requirement to PHP 7.1, because security support for PHP 7.0 actually runs out before PHP 5.6 does
But it is not up to me to make that decision.
@frankmayer In the past (and currently) there is a dynamic between what hosters are supporting (influenced by what's available and supported in standard distros) vs what developers want. Personally, I would love to see J3.7 drop php5.3 but that likely won't happen. On the other hand J4 is being developed with a php5.6+ design. Sadly most hosters are still hesitant to fully support php 7 even when they offer the option to use php7 (when problems arise they typically tell people to go back to using php5.6, even some developers of extensions tell customers that)
Balancing the hoster support, 3rd party extensions, and platform needs is an unpleasant challenge. IMO php5.6+ is a good compromise for J4.
From what I could grasp by reviewing the code alone, all looks good.
There are some areas, like database queries without a try / catch block neither before or after the optimisation, but I guess that alone might be a thing for different PR. Anonther thing that caught my attention is this passage from JApplicationSite::initialiseApp().
Good work, thanks!
@matrikular You're very welcome! I always wanted to give back to the Joomla project, and decided to give it a try by doing this. And as I mentioned earlier, if you (as the steering users) are OK with those kinds of changes I am open to expand the scope of optimizations to the whole project (minus the vendor imports... as I learned
I have tested this item
on code review
Fixed some conflicts that had occurred in the meantime. It's essentially ready to be merged, when the team finds the time and PR is OK.
I have tested this item
code review
conflicts
@andrepereiradasilva Yes, but after the referenced open PR's are merged. Don't want to do this another three times ;)
right i see
Edited initial post to show which sub-PRs still have to go through, in order to make this one easier to digest.
It would be great if the initial big batch PRs of mine could make it into alpha2. Thanks!
I have tested this item
code review
Just an idea...
If there is a considerable performance in joomla paging loading maybe a metric could be done with and without all these performance prs and if is enough it could be advertises in joomla 3.8 release campaign?
thanks frankmayer for your work!
@peteruoi that is an interesting idea. However I assume that any improvements from all the PRs will vary a lot with mileage. Unfortunately I haven't got the time to set this up and do the tests.
Oh, and a lot of things made it already into 3.7.
Another situation where I would hope to see some measurable optimization would be, if the indexer performance PR would be merged (see #13511).
This would of course mostly be measureable with initial (or re-) indexing of a lot of index-able content.
But I also would like to think that on changing of content, it might work a little bit faster.
Bump @andrepereiradasilva & @Quy Problem solved. Pls test (review changes) to go for RTC
I have tested this item
Code review
@peteruoi when we have levels like Joomla server side performance things are not like 30% Faster in all Pages. Is really relative to each change and to the amount of scenarios that change apllies to
For instance in the past made several changes that boosted performance only in multilingual sites.
Imo the most important performance improvements are the ones done to the critical page that apllies to all pages and also most used parts (com_content, jroute, etc).
That said general improvements like this set of prs also contribute as a whole.
I have tested this item
Code review.
Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
@andrepereiradasilva Even with a 5% percent general it will be awesome news for advertisment for 3.8 campaign. And no one forbids us to say 5% to 20% (at miltilingual for example).
I think it's a pity that the optimization you awesome fellows do and make joomla and the internet faster is not recognized enough :).
This metric of course it cannot be pr dependent but before release 3.7 vs 3.8 tests and if they are favourite for us we advertise ;)
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-15 06:48:00 |
Closed_By | ⇒ | rdeutz | |
Labels |
Added:
?
|
Hooray, this really big chunk was finally merged! Thank you all for your comments and efforts in testing
Travis error is:
https://travis-ci.org/joomla/joomla-cms/jobs/162592381