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
Joomla String Class Optimisation #6787
Conversation
…require_once calls.
…direct usage of JString::is_ascii() instead (JApplicationWeb and JApplication both imported the utf8_is_ascii() function directly from the phputf8 library and it seemed to cause a cannot redeclare function error).
Hi Thanks for this PR! Anything that touches the framework files should be made to the string package in the framework repository (https://github.com/joomla-framework/string) If you're happy to do that it would be really appreciated :) Thanks! |
Thanks George, That was going to be my original plan once I noticed it was one of the framework packages...however the current version of the package we're using in Joomla doesn't seem to match up with the Master String branch in the framework repository (as an example, in the CMS there's just a String.php file, but in the framework String.php defers to another file called StringHelper.php which actually contains the function definitions...since I didn't know when/if those updates would be brought into the CMS I thought it would be better to create my PR against what is currently in the CMS first). Maybe you have some extra info about whether or not the String package is going to be updated in the CMS soon? If it will be, I can certainly create an additional PR against the framework code. |
We will be merging the string updates into 3.5 - as there is a low chance of a b/c break because of the class rename (PHP 7 forbid us to use the word string as it's now protected). #6600 The PR is already in for it though |
Thanks for the update...I'll work on the PR against the Framework package sometime today ;-). |
I'm going to go ahead and close this PR since Michael merged in the separate PR I created for the String package earlier today. |
In testing I had done over the weekend and then again today, I was noticing that the String.php file (which is in the Joomla String package in the libraries/vendors folder) was exhibiting some odd performance behavior on my Windows machine.
Hopefully others will be able to confirm the same thing happening on their end and can confirm that this PR fixes the issue.
Overview of the Issue:
Basically I noticed that in the Joomla\String\String class a good majority of the methods have a "require_once" included in them and this seems to slow down the usage of the function over repeated calls.
Additionally, while coming up with some code that could be used by testers to see the before/after effects on their end, I noticed another issue crop up which has to do with an older call to jimport() in JApplicationWeb and JApplication so the last commit I have that changes those files is to fix that issue.
Details on Performance Improvements:
These examples are using my testing code I'm including below and cause 500 executions of each of the functions.
Before the String class optimizations (Total Time ~502 ms):
After the String class optimizations (Total Time ~101 ms):
Testing Instructions
Go into your Joomla Administrator and turn on debugging (so that you can see the profile information on the page).
Once enabled and you can see the profiler info, go ahead and copy the following lines of code into your administrator/index.php file (you can copy these lines in right after line 44...right after this line: "$app = JFactory::getApplication('administrator');")
Once the code below is copied in, go ahead and test and take note of the before timings.
Then apply the patch and observe the after timings.